Trivial fixes for some IDENTIFICATION comment lines

Started by Shulgin, Oleksandrabout 10 years ago4 messageshackers
Jump to latest
#1Shulgin, Oleksandr
oleksandr.shulgin@zalando.de

Hello,

I've noticed that in src/backend/replication/logical/reorderbuffer.c, the
IDENTIFICATION comment line is incorrect:

* IDENTIFICATION
* src/backend/replication/reorderbuffer.c

By using a simple find+grep command I can see this is also the case for the
following files:

$ find src -name \*.c -o -name \*.h | xargs grep -A1 IDENTIFICATION | grep
-v -E 'IDENTIFICATION|--' | grep -v '^\(src/.*\.[ch]\)-\s*\*\s*\1\s*$'

src/include/utils/evtcache.h- * src/backend/utils/cache/evtcache.c
src/backend/utils/cache/relfilenodemap.c- *
src/backend/utils/cache/relfilenode.c
src/backend/utils/adt/version.c- *
src/backend/storage/ipc/dsm_impl.c- * src/backend/storage/ipc/dsm.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/logical/logicalfuncs.c- *
src/backend/replication/logicalfuncs.c
src/backend/commands/dropcmds.c- * src/backend/catalog/dropcmds.c

The one wtih version.c is a false positive: there's just an extra blank
line in the comment.

A patch to fix the the above is attached.

Another minor thing is that if there is a convention for the order of
Copyright, NOTES and IDENTIFICATION, then it is not followed strictly.
Compare e.g. reorderbuffer.c vs. snapbuild.c.

--
Alex

Attachments:

0001-Assorted-IDENTIFICATION-comment-line-fixes.patchtext/x-patch; charset=US-ASCII; name=0001-Assorted-IDENTIFICATION-comment-line-fixes.patchDownload+7-8
#2Andres Freund
andres@anarazel.de
In reply to: Shulgin, Oleksandr (#1)
Re: Trivial fixes for some IDENTIFICATION comment lines

On 2016-01-18 12:52:06 +0100, Shulgin, Oleksandr wrote:

I've noticed that in src/backend/replication/logical/reorderbuffer.c, the
IDENTIFICATION comment line is incorrect:

* IDENTIFICATION
- * src/backend/catalog/dropcmds.c
- * src/backend/replication/logicalfuncs.c
- * src/backend/replication/reorderbuffer.c
- * src/backend/replication/snapbuild.c
- * src/backend/storage/ipc/dsm.c
- * src/backend/utils/cache/relfilenode.c
- * src/backend/utils/cache/evtcache.c

How about we simply drop them instead alltogether? This isn't exactly a
seldomly made mistake, and they seem to actually contribute very little
value?

Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#2)
Re: Trivial fixes for some IDENTIFICATION comment lines

Andres Freund <andres@anarazel.de> writes:

On 2016-01-18 12:52:06 +0100, Shulgin, Oleksandr wrote:

I've noticed that in src/backend/replication/logical/reorderbuffer.c, the
IDENTIFICATION comment line is incorrect:

* IDENTIFICATION
- * src/backend/catalog/dropcmds.c
- * src/backend/replication/logicalfuncs.c
- * src/backend/replication/reorderbuffer.c
- * src/backend/replication/snapbuild.c
- * src/backend/storage/ipc/dsm.c
- * src/backend/utils/cache/relfilenode.c
- * src/backend/utils/cache/evtcache.c

How about we simply drop them instead alltogether? This isn't exactly a
seldomly made mistake, and they seem to actually contribute very little
value?

Personally I think they're of some value. Particularly with stuff like
Makefiles, which are otherwise confusingly similar.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#3)
Re: Trivial fixes for some IDENTIFICATION comment lines

On Mon, Jan 18, 2016 at 12:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andres Freund <andres@anarazel.de> writes:

On 2016-01-18 12:52:06 +0100, Shulgin, Oleksandr wrote:

I've noticed that in src/backend/replication/logical/reorderbuffer.c, the
IDENTIFICATION comment line is incorrect:

* IDENTIFICATION
- * src/backend/catalog/dropcmds.c
- * src/backend/replication/logicalfuncs.c
- * src/backend/replication/reorderbuffer.c
- * src/backend/replication/snapbuild.c
- * src/backend/storage/ipc/dsm.c
- * src/backend/utils/cache/relfilenode.c
- * src/backend/utils/cache/evtcache.c

How about we simply drop them instead alltogether? This isn't exactly a
seldomly made mistake, and they seem to actually contribute very little
value?

Personally I think they're of some value. Particularly with stuff like
Makefiles, which are otherwise confusingly similar.

I'm not unwilling to keep them around, but I tend to agree with
Andres. My editor will happily tell me which file I'm editing, and it
will do so regardless of whether I am on the first page of the file or
the hundredth page. The IDENTIFICATION comment isn't present in every
file and is occasionally wrong and in any event isn't displayed except
at the top. So I find those comments a fairly useless nuisance --
it's just one more thing I have to adjust when adding a contrib module
or a directory someplace, or moving something around. However, if you
want to keep them around, I don't have a big problem with that. It's
not necessary for everyone to find exactly the same things useful that
I do.

(vim FTW!)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers