From: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
---|---|
To: | Corey Huinker <corey(dot)huinker(at)gmail(dot)com> |
Cc: | PostgreSQL <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: \if, \elseif, \else, \endif (was Re: PSQL commands: \quit_if, \quit_unless) |
Date: | 2017-02-01 15:33:28 |
Message-ID: | alpine.DEB.2.20.1702011631060.31128@lancre |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
<sorry, resent, wrong from again *$%(at)#&!?>
Hello Corey,
>> There is a spurious empty line added at the very end of "mainloop.h":
>>
>> +
>> #endif /* MAINLOOP_H */
>
> Not in my diff, but that's been coming and going in your diff reviews.
Strange. Maybe this is linked to the warning displayed with "git apply" when
I apply the diff.
>> I would suggest to add a short one line comment before each test to
>> explain what is being tested, like "-- test \elif execution", "-- test
>> \else execution"...
>
> Where are you suggesting this?
In "regress/sql/psql.sql", in front of each group which starts a test.
>> Debatable suggestion about "psql_branch_empty":
>
> The name isn't great. Maybe psql_branch_stack_empty()?
Yep, maybe, or "empty_stack" or "stack_is_empty" or IDK...
> "psql_branch_end_state": it is a pop, it could be named "psql_branch_pop"
>
> Yeah, we either need to go fully with telling the programmer that it's a
> stack (push/pop/empty) or (begin_branch/end_branch/not_branching). I'm
> inclined to go full-stack, as it were.
Anything consistent is ok. I'm fine with calling a stack a stack:-)
--
Fabien.
From | Date | Subject | |
---|---|---|---|
Next Message | Konstantin Knizhnik | 2017-02-01 15:40:33 | Re: Deadlock in XLogInsert at AIX |
Previous Message | Vik Fearing | 2017-02-01 15:18:03 | Re: new autovacuum criterion for visible pages |