On Thu, Apr 16, 2026 at 4:26 AM Peter Eisentraut <[email protected]> wrote:
>
> On 15.04.26 09:41, Peter Smith wrote:
> > On Wed, Apr 15, 2026 at 3:48 PM Xiaopeng Wang <[email protected]> wrote:
> >>
> > ...
> >>
> >> Looks good to me.
> >>
> >> A small comment is that, the commit message claims only “add missing 
> >> period to DETAIL messages”, but apparently the patch also changes 
> >> capitalization in many places, so the commit message should be updated.
> >>
> >
> > Fair point. Thanks for your review!
> >
> > PSA v4 which has an improved commit message.
>
> Most of these look good, but I don't think this is an improvement:
>
> -DETAIL:  syntax error at end of input
> +DETAIL:  syntax error at end of input.
>
> The guidelines say that the detail message should be a sentence.  But
> this is not a sentence.  Just adding a period doesn't make it a
> sentence.  IMO, having a period at the end of a thing that is not a
> sentence is worse than not having it be a sentence.  The latter just
> violates our quality standards, the former is confusing for a user who
> sees that particular output.

Thanks for your review.

OK, I've removed that "syntax error at end of input." change from the patch.

IIUC, the identical (non-sentence) issue also applies to other ones
like 'syntax error at or near \"%s\".'
So I  removed those from the patch, too.

PSA v5, which makes the above changes.

~~

Now I am having doubts about those '%s depends on column \"%s\".' changes.

Despite looking like sentences, I have no control over the first word
capitalisation, so the results are like:
-DETAIL:  rule _RETURN on view usersview depends on column "seq"
+DETAIL:  rule _RETURN on view usersview depends on column "seq".

In hindsight, maybe just adding periods for these was also not
warranted? What do you think about those?

======
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachment: v5-0001-Change-DETAIL-messages-to-conform-to-the-style-gu.patch
Description: Binary data

Reply via email to