Re: Yet another small patch - reorderbuffer.c:1099

On Tue, Apr 5, 2016 at 10:38 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> IMO the code is wrong.  There should be a single block comment saying
> something like "Remove the node from its containing list.  In the FOO
> case, the list corresponds to BAR and therefore we delete it because
> BAZ.  In the QUUX case the list is PLUGH and we delete in because THUD."
> Then a single line dlist_delete(...) follows.
>
> The current arrangement looks bizantine to me, for no reason.  If we
> think that one of the two branches might do something additional to the
> list deletion, surely that will be in a separate stanza with its own
> comment; and if we ever want to remove the list deletion from one of the
> two cases (something that strikes me as unlikely) then we will need to
> fix the comment, too.

+1 to everything here except for the way byzantine is spelled.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company