Thread: Typo: llvm*.cpp files identified as llvm*.c
Hi, Attached find a patch to fix $subject. Thanks, Amit
Attachment
On Tue, Jan 22, 2019 at 01:43:32PM +0900, Amit Langote wrote: > Hi, > > Attached find a patch to fix $subject. It is like that since 31bc604. Andres, would you take care of it? It is your commit after all.. -- Michael
Attachment
Hi, On 2019-01-22 13:43:32 +0900, Amit Langote wrote: > Attached find a patch to fix $subject. Thanks, pushed to master and 11. Greetings, Andres Freund
On 2019/01/23 4:51, Andres Freund wrote: > Hi, > > On 2019-01-22 13:43:32 +0900, Amit Langote wrote: >> Attached find a patch to fix $subject. > > Thanks, pushed to master and 11. Thank you. Regards, Amit
On Wed, Jan 23, 2019 at 1:16 PM Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> On 2019/01/23 4:51, Andres Freund wrote:
> > On 2019-01-22 13:43:32 +0900, Amit Langote wrote:
> >> Attached find a patch to fix $subject.
> >
> > Thanks, pushed to master and 11.
>
> Thank you.
It's not only the ending that's wrong. Here are some more source
files whose IDENTIFICATION heading doesn't exactly match their path:
$ git grep -A 1 IDENTIFICATION | grep -v IDENTIFICATION | grep -v --
-- | sed 's/-[^a-z][^a-z]*/ /' | awk '{ if ($1 != $2) print; }'
doc/src/sgml/lobj.sgml src/test/examples/testlo.c
src/backend/catalog/pg_publication.c pg_publication.c
src/backend/commands/publicationcmds.c publicationcmds.c
src/backend/commands/subscriptioncmds.c subscriptioncmds.c
src/backend/jit/llvm/llvmjit_inline.cpp
src/backend/lib/llvmjit/llvmjit_inline.cpp
src/backend/jit/llvm/llvmjit_wrap.cpp src/backend/lib/llvm/llvmjit_wrap.cpp
src/backend/optimizer/util/appendinfo.c src/backend/optimizer/path/appendinfo.c
src/backend/optimizer/util/inherit.c src/backend/optimizer/path/inherit.c
src/backend/replication/logical/logicalfuncs.c
src/backend/replication/logicalfuncs.c
src/backend/replication/logical/reorderbuffer.c
src/backend/replication/reorderbuffer.c
src/backend/replication/logical/snapbuild.c src/backend/replication/snapbuild.c
src/backend/replication/pgoutput/Makefile src/backend/replication/pgoutput
src/backend/utils/adt/version.c
src/include/replication/pgoutput.h pgoutput.h
This could be really confusing for erm, future people reading a dot
matrix print-out of the source code?
--
Thomas Munro
http://www.enterprisedb.com
On Wed, Jan 23, 2019 at 01:37:41PM +1300, Thomas Munro wrote:
> It's not only the ending that's wrong. Here are some more source
> files whose IDENTIFICATION heading doesn't exactly match their path:
Good point.
> $ git grep -A 1 IDENTIFICATION | grep -v IDENTIFICATION | grep -v --
> -- | sed 's/-[^a-z][^a-z]*/ /' | awk '{ if ($1 != $2) print; }'
> doc/src/sgml/lobj.sgml src/test/examples/testlo.c
Some noise?
> src/backend/catalog/pg_publication.c pg_publication.c
> src/backend/commands/publicationcmds.c publicationcmds.c
> src/backend/commands/subscriptioncmds.c subscriptioncmds.c
> src/backend/jit/llvm/llvmjit_inline.cpp
> src/backend/lib/llvmjit/llvmjit_inline.cpp
> src/backend/jit/llvm/llvmjit_wrap.cpp src/backend/lib/llvm/llvmjit_wrap.cpp
> src/backend/replication/logical/logicalfuncs.c
> src/backend/replication/logicalfuncs.c
> src/backend/replication/logical/reorderbuffer.c
> src/backend/replication/reorderbuffer.c
> src/backend/replication/logical/snapbuild.c src/backend/replication/snapbuild.c
> src/backend/replication/pgoutput/Makefile src/backend/replication/pgoutput
> src/include/replication/pgoutput.h pgoutput.h
Logical decoding, JIT and logical replication.
> src/backend/optimizer/util/appendinfo.c src/backend/optimizer/path/appendinfo.c
> src/backend/optimizer/util/inherit.c src/backend/optimizer/path/inherit.c
These two are recent, from b60c397.
> src/backend/utils/adt/version.c
This complains about the indentation?
> This could be really confusing for erm, future people reading a dot
> matrix print-out of the source code?
Yes, it would be nice to make all that consistent. Perhaps the
authors of the related commits would prefer fix that themselves?
--
Michael
Attachment
Hi Thomas,
On 2019/01/23 9:37, Thomas Munro wrote:
> On Wed, Jan 23, 2019 at 1:16 PM Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> On 2019/01/23 4:51, Andres Freund wrote:
>>> On 2019-01-22 13:43:32 +0900, Amit Langote wrote:
>>>> Attached find a patch to fix $subject.
>>>
>>> Thanks, pushed to master and 11.
>>
>> Thank you.
>
> It's not only the ending that's wrong. Here are some more source
> files whose IDENTIFICATION heading doesn't exactly match their path:
>
> $ git grep -A 1 IDENTIFICATION | grep -v IDENTIFICATION | grep -v --
> -- | sed 's/-[^a-z][^a-z]*/ /' | awk '{ if ($1 != $2) print; }'
> doc/src/sgml/lobj.sgml src/test/examples/testlo.c
> src/backend/catalog/pg_publication.c pg_publication.c
> src/backend/commands/publicationcmds.c publicationcmds.c
> src/backend/commands/subscriptioncmds.c subscriptioncmds.c
> src/backend/jit/llvm/llvmjit_inline.cpp
> src/backend/lib/llvmjit/llvmjit_inline.cpp
> src/backend/jit/llvm/llvmjit_wrap.cpp src/backend/lib/llvm/llvmjit_wrap.cpp
> src/backend/optimizer/util/appendinfo.c src/backend/optimizer/path/appendinfo.c
> src/backend/optimizer/util/inherit.c src/backend/optimizer/path/inherit.c
(I introduced this one.)
> src/backend/replication/logical/logicalfuncs.c
> src/backend/replication/logicalfuncs.c
> src/backend/replication/logical/reorderbuffer.c
> src/backend/replication/reorderbuffer.c
> src/backend/replication/logical/snapbuild.c src/backend/replication/snapbuild.c
> src/backend/replication/pgoutput/Makefile src/backend/replication/pgoutput
> src/backend/utils/adt/version.c
> src/include/replication/pgoutput.h pgoutput.h
Oops. In fact, my colleague just pointed that out to me about the files I
sent a patch upthread for.
> This could be really confusing for erm, future people reading a dot
> matrix print-out of the source code?
Heh. Actually, I'm not sure which existing tools these IDENTIFICATION
lines are for, but maybe someone somewhere relies on these. Or maybe not,
because they'd have noticed by now. :)
Thanks,
Amit
Hi, On 2019-01-23 09:55:22 +0900, Michael Paquier wrote: > On Wed, Jan 23, 2019 at 01:37:41PM +1300, Thomas Munro wrote: > > This could be really confusing for erm, future people reading a dot > > matrix print-out of the source code? I think we should just rip them out. It's useless noise. > Yes, it would be nice to make all that consistent. Perhaps the > authors of the related commits would prefer fix that themselves? That sounds like a waste of effort. This is fixing up useless anachronisms, I don't understand what'd be gained by splitting this up over multiple people. Greetings, Andres Freund
Dear Hackers.
> -----Original Message-----
> From: Thomas Munro [mailto:thomas.munro@enterprisedb.com]
> Sent: Wednesday, January 23, 2019 9:38 AM
> To: Amit Langote
> Cc: Andres Freund; Pg Hackers
> Subject: Re: Typo: llvm*.cpp files identified as llvm*.c
>
> On Wed, Jan 23, 2019 at 1:16 PM Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> > On 2019/01/23 4:51, Andres Freund wrote:
> > > On 2019-01-22 13:43:32 +0900, Amit Langote wrote:
> > >> Attached find a patch to fix $subject.
> > >
> > > Thanks, pushed to master and 11.
> >
> > Thank you.
>
> It's not only the ending that's wrong. Here are some more source files whose IDENTIFICATION heading doesn't exactly
match
> their path:
I found the same problem as you while checking the typo patch.
So I attached a patch that modified the correct file path in IDENTIFICATION.
Regards.
Moon.
>
> $ git grep -A 1 IDENTIFICATION | grep -v IDENTIFICATION | grep -v --
> -- | sed 's/-[^a-z][^a-z]*/ /' | awk '{ if ($1 != $2) print; }'
> doc/src/sgml/lobj.sgml src/test/examples/testlo.c src/backend/catalog/pg_publication.c pg_publication.c
> src/backend/commands/publicationcmds.c publicationcmds.c src/backend/commands/subscriptioncmds.c subscriptioncmds.c
> src/backend/jit/llvm/llvmjit_inline.cpp
> src/backend/lib/llvmjit/llvmjit_inline.cpp
> src/backend/jit/llvm/llvmjit_wrap.cpp src/backend/lib/llvm/llvmjit_wrap.cpp
> src/backend/optimizer/util/appendinfo.c src/backend/optimizer/path/appendinfo.c
> src/backend/optimizer/util/inherit.c src/backend/optimizer/path/inherit.c
> src/backend/replication/logical/logicalfuncs.c
> src/backend/replication/logicalfuncs.c
> src/backend/replication/logical/reorderbuffer.c
> src/backend/replication/reorderbuffer.c
> src/backend/replication/logical/snapbuild.c src/backend/replication/snapbuild.c
> src/backend/replication/pgoutput/Makefile src/backend/replication/pgoutput src/backend/utils/adt/version.c
> src/include/replication/pgoutput.h pgoutput.h
>
> This could be really confusing for erm, future people reading a dot matrix print-out of the source code?
>
> --
> Thomas Munro
> http://www.enterprisedb.com
Attachment
On Wed, Jan 23, 2019 at 2:02 PM Andres Freund <andres@anarazel.de> wrote: > On 2019-01-23 09:55:22 +0900, Michael Paquier wrote: > > On Wed, Jan 23, 2019 at 01:37:41PM +1300, Thomas Munro wrote: > > > This could be really confusing for erm, future people reading a dot > > > matrix print-out of the source code? > > I think we should just rip them out. It's useless noise. +1 IDENTIFICATION used to show magic CVS keywords, which was standard practice back then and probably useful. Now it's just a useless path, because Magnus removed the keywords 10 years ago in commit 9f2e2113869. The function name comments are similar, though less consistent so I'm too lazy to write a script to find one that is actually wrong (with which to trigger Andres's let's-delete-them-all response :-D). At least in the planner code, I think those might originally have been documentation of the names of the Lisp functions that each C function was ported from (for example in 4.2 source, sort_inner_and_outer's comment says "sort-inner-and-outer", a Lispy spelling). Bruce delispified some of them 20 years ago in commit 6724a507874. -- Thomas Munro http://www.enterprisedb.com
On 2019-01-23 14:43:15 +1300, Thomas Munro wrote: > The function name comments are similar, though less consistent so I'm > too lazy to write a script to find one that is actually wrong (with > which to trigger Andres's let's-delete-them-all response :-D). I wish function comment styles were more consistent, but there's *SO* many styles, that I think it's hard to nicely automate it. And it's much more likely to cause conflicts than removing IDENTIFICATION. So... Greetings, Andres Freund
On Tue, Jan 22, 2019 at 05:49:46PM -0800, Andres Freund wrote: > On 2019-01-23 14:43:15 +1300, Thomas Munro wrote: >> The function name comments are similar, though less consistent so I'm >> too lazy to write a script to find one that is actually wrong (with >> which to trigger Andres's let's-delete-them-all response :-D). I am not sure if anybody uses them for anything automatically, still I find myself from time to time looking at them to remember on which path the file is located when opened in emacs. So I still like having those references, perhaps I am just a minority. Being in the minority is usually a cool thing, still if you wish ripping all these out it's not like I'll cry for that, so please feel free to do as you see fit. > I wish function comment styles were more consistent, but there's *SO* > many styles, that I think it's hard to nicely automate it. And it's much > more likely to cause conflicts than removing IDENTIFICATION. So... Yes, I am usually more annoyed by the inconsistency of the function upper blocks than IDENTIFICATION... So I just try to stick with keeping any new code consistent with the surroundings. Making back-patching harder than it is now is not really appealing, so I'd be -1 for doing any consistency work. Patching six branches for the same patch is already a lot of work. -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes:
> I am not sure if anybody uses them for anything automatically, still I
> find myself from time to time looking at them to remember on which
> path the file is located when opened in emacs. So I still like having
> those references, perhaps I am just a minority.
FWIW, I like having them too, though I agree it's not super important.
regards, tom lane
>>>>> "Andres" == Andres Freund <andres@anarazel.de> writes: [IDENTIFICATION] Andres> I think we should just rip them out. It's useless noise. +1 -- Andrew (irc:RhodiumToad)
Hi, On 2019-01-23 12:01:10 +0900, Michael Paquier wrote: > On Tue, Jan 22, 2019 at 05:49:46PM -0800, Andres Freund wrote: > > On 2019-01-23 14:43:15 +1300, Thomas Munro wrote: > >> The function name comments are similar, though less consistent so I'm > >> too lazy to write a script to find one that is actually wrong (with > >> which to trigger Andres's let's-delete-them-all response :-D). > > I am not sure if anybody uses them for anything automatically, still I > find myself from time to time looking at them to remember on which > path the file is located when opened in emacs. I often want to know that too - isn't it easier to just meta-x f and see the directory displayed? Because that doesn't require jumping to the head of the file... Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2019-01-23 12:01:10 +0900, Michael Paquier wrote:
>> I am not sure if anybody uses them for anything automatically, still I
>> find myself from time to time looking at them to remember on which
>> path the file is located when opened in emacs.
> I often want to know that too - isn't it easier to just meta-x f and see
> the directory displayed? Because that doesn't require jumping to the
> head of the file...
My own workflow often involves editing copies that are not in the original
directory, so that answer doesn't work for me. In general it won't work
anytime you are looking at a file out-of-context.
regards, tom lane
On Wed, Jan 23, 2019 at 7:24 PM Andrew Gierth <andrew@tao11.riddles.org.uk> wrote: > >>>>> "Andres" == Andres Freund <andres@anarazel.de> writes: > > [IDENTIFICATION] > > Andres> I think we should just rip them out. It's useless noise. > > +1 Here's a patch like that. 948 files changed, 3215 deletions(-) -- Thomas Munro https://enterprisedb.com