On Tue, Jul 16, 2019 at 10:02 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Tue, Jul 16, 2019 at 4:43 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > On Tue, Jul 16, 2019 at 2:09 AM Robert Haas <robertmhaas@gmail.com> wrote:
> > > This patch has some problems with naming consistency. There's a
> > > function called PushUndoRequest() which calls a function called
> > > RegisterRollbackReq() to do the heart of the work. So, is it undo or
> > > rollback? Are we pushing or registering? Is it a request or a req?
> >
> > I think we can rename PushUndoRequest as RegisterUndoRequest and
> > RegisterRollbackReq as RegisterUndoRequestGuts.
>
> One thing I am not sure about the above suggestion is whether it is a
> good idea to expose a function which ends with 'Guts'. I have checked
> and found that there are a few similar precedents like
> ExecuteTruncateGuts. Another idea could be to rename
> RegisterRollbackReq as RegisterUndoRequestInternal. We have few
> precedents for that as well.
I don't personally like Guts, not only because bringing human (or
animal) body parts into this seems unnecessary, but more importantly
because it's not at all descriptive. Internal is no better. The point
is that you need to give the functions names that make it clear how
what one function does is different from what another function does,
and neither Guts nor Internal is going to help with that.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
