Re: [PATCH] New [relation] option engine
Alvaro Herrera <alvherre@alvh.no-ip.org>
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Nikolay Shaplov <dhyan@nataraj.su>
Cc: Andres Freund <andres@anarazel.de>, pgsql-hackers@lists.postgresql.org, Michael Paquier <michael@paquier.xyz>, Amit Langote <amitlangote09@gmail.com>
Date: 2022-05-15T12:25:47Z
Lists: pgsql-hackers
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Tighten test_predtest's input checks, and improve error messages.
- 00ac25a3c365 17.0 cited
-
Avoid reference to nonexistent array element in ExecInitAgg().
- 92957ed98c5c 16.0 cited
-
Clean up newlines following left parentheses
- c4f113e8fef9 15.0 cited
I'm sorry if you've already said this elsewhere, but can you please state what is the *intention* of this patchset? If it's a pure refactoring (but I don't think it is), then it's a net loss, because after pgindent it summarizes as: 58 files changed, 2714 insertions(+), 2368 deletions(-) so we end up 500+ lines worse than the initial story. However, I suspect that's not the final situation, since I saw a comment somewhere that opclass options have to be rewritten to modify this mechanism, and I suspect that will remove a few lines. And you maybe have a more ambitious goal. But what is it? Please pgindent your code for the next submission, making sure to add your new typedef(s) to typedefs.list so that it doesn't generate stupid spaces. After pgindenting you'll notice the argument lists of some functions look bad (cf. commit c4f113e8fef9). Please fix that too. I notice that you kept the commentary about lock levels in the place where they were previously defined. This is not good. Please move each explanation next to the place where each option is defined. For next time, please use "git format-patch" for submission, and write a tentative commit message. The committer may or may not use your proposed commit message, but with it they will know what you're trying to achieve. The translatability marker for detailmsg for enums is wrong AFAICT. You need gettext_noop() around the strings themselves IIRC. I think you need to get rid of the _() call around the variable that receives that value and use errdetail() instead of errdetail_internal(), to avoid double-translating it; but I'm not 100% sure. Please experiment with "make update-po" until you get the messages in the .po file. You don't need braces around single-statement blocks. Thanks -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "World domination is proceeding according to plan" (Andrew Morton)