This is a review for patch
CAApHDvpAgtyPZB2kwa0MmTkSaYG9+vasHYjmjFaTNgXr1aDC4Q@mail.gmail.com
The patch is readable, applies fine and builds without warnings.
It contains sufficient documentation.
It works as it should, no crashes or errors.
It is well written, in fact it improves the readability of
the log_line_prefix function in backend/utils/error/elog.c.
I think that this is a useful feature as it can help to make
stderr logging more readable for humans.
I have a few gripes with the English:
- In the documentation patch:
+ numeric literal after the % and before the option. A negative + padding will cause the status
informationto be padded on the + right with spaces to give it a minimum width. Whereas a positive +
paddingwill pad on the left. Padding can be useful keep log + files more human readable.
I think that there should be a comma, not a period, before "whereas".
- The description for the function process_log_prefix_padding should probably mention that it returns NULL if it
encountersbad syntax.
- This comment:
+ /* Format is invalid if it ends with the padding number */
should begin with lower case.
- In this comment:
+ /* + * Process any formatting which may exist after the '%'. + * Note
thatthis moves p passed the padding number + * if it exists. + */
It should be "past", not "passed".
If these details get fixed, I'll mark the patch ready for committer.
Yours,
Laurenz Albe
