Re: boolin comment not moved when code was refactored
On Fri, Oct 20, 2023 at 2:31 AM Tom Lane wrote: > > Peter Smith writes: > > PSA v2. > > Pushed. > Thanks for pushing. == Kind Regards, Peter Smith. Fujitsu Australia
Re: boolin comment not moved when code was refactored
Peter Smith writes: > PSA v2. Pushed. regards, tom lane
Re: boolin comment not moved when code was refactored
On Thu, Oct 19, 2023 at 3:26 PM Vik Fearing wrote: > > On 10/19/23 06:17, Peter Smith wrote: > >> In short, maybe the whole comment should just be > >> > >> /* > >> * boolin - input function for type boolean > >> */ > >> > > How about "boolin - converts a boolean string value to 1 or 0" > > > Personally, I do not like exposing the implementation of a boolean (it > is a base type that is not a numeric), so I prefer Tom's suggestion. OK. Done that way. PSA v2. == Kind Regards, Peter Smith. Fujitsu Australia v2-0001-Relocate-or-remove-stale-comments.patch Description: Binary data
Re: boolin comment not moved when code was refactored
On 10/19/23 06:17, Peter Smith wrote: In short, maybe the whole comment should just be /* * boolin - input function for type boolean */ How about "boolin - converts a boolean string value to 1 or 0" Personally, I do not like exposing the implementation of a boolean (it is a base type that is not a numeric), so I prefer Tom's suggestion. -- Vik Fearing
Re: boolin comment not moved when code was refactored
On Thu, Oct 19, 2023 at 2:55 PM Tom Lane wrote: > > Richard Guo writes: > > On Thu, Oct 19, 2023 at 10:35 AM Peter Smith wrote: > >> I happened upon a function comment referring to non-existent code > >> (that code was moved to another location many years ago). > >> > >> Probably better to move that comment too. Thoughts? > > > Agreed. +1 to move that comment. > > Hm, I'm inclined to think that the comment lines just above: > > *boolin- converts "t" or "f" to 1 or 0 > * > * Check explicitly for "true/false" and TRUE/FALSE, 1/0, YES/NO, ON/OFF. > * Reject other values. > > are also well past their sell-by date. The one-line summary > "converts "t" or "f" to 1 or 0" is not remotely accurate anymore. > Perhaps we should just drop it? Or else reword to something > vaguer, like "input function for boolean". The "Check explicitly" > para no longer describes logic in this function. We could move > it to parse_bool_with_len, but that seems to have a suitable > comment already. > Yes, I had the same thought about the rest of the comment being outdated but just wanted to test the water to see if a small change was accepted before I did too much. > In short, maybe the whole comment should just be > > /* > * boolin - input function for type boolean > */ > How about "boolin - converts a boolean string value to 1 or 0" == Kind Regards, Peter Smith. Fujitsu Australia.
Re: boolin comment not moved when code was refactored
Richard Guo writes: > On Thu, Oct 19, 2023 at 10:35 AM Peter Smith wrote: >> I happened upon a function comment referring to non-existent code >> (that code was moved to another location many years ago). >> >> Probably better to move that comment too. Thoughts? > Agreed. +1 to move that comment. Hm, I'm inclined to think that the comment lines just above: *boolin- converts "t" or "f" to 1 or 0 * * Check explicitly for "true/false" and TRUE/FALSE, 1/0, YES/NO, ON/OFF. * Reject other values. are also well past their sell-by date. The one-line summary "converts "t" or "f" to 1 or 0" is not remotely accurate anymore. Perhaps we should just drop it? Or else reword to something vaguer, like "input function for boolean". The "Check explicitly" para no longer describes logic in this function. We could move it to parse_bool_with_len, but that seems to have a suitable comment already. In short, maybe the whole comment should just be /* * boolin - input function for type boolean */ Agreed with your original point, though. regards, tom lane
Re: boolin comment not moved when code was refactored
On Thu, Oct 19, 2023 at 10:35 AM Peter Smith wrote: > Hi. > > I happened upon a function comment referring to non-existent code > (that code was moved to another location many years ago). > > Probably better to move that comment too. Thoughts? Agreed. +1 to move that comment. Thanks Richard
boolin comment not moved when code was refactored
Hi. I happened upon a function comment referring to non-existent code (that code was moved to another location many years ago). Probably better to move that comment too. Thoughts? PSA. == Kind Regards, Peter Smith. Fujitsu Australia v1-0001-Relocate-old-comment.patch Description: Binary data
