Postgres Pro
Downloads
Home   >   mailing lists

Thread: Trivial fixes for some IDENTIFICATION comment lines

Trivial fixes for some IDENTIFICATION comment lines

From
"Shulgin, Oleksandr"
Date:
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

Attachment

Re: Trivial fixes for some IDENTIFICATION comment lines

From
Andres Freund
Date:
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

Re: Trivial fixes for some IDENTIFICATION comment lines

From
Tom Lane
Date:
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

Re: Trivial fixes for some IDENTIFICATION comment lines

From
Robert Haas
Date:
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