Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Mark Kettenis <mark.kettenis@xs4all.nl>
To: eliz@gnu.org
Cc: gdb-patches@sources.redhat.com
Subject: Re: [COMMIT] Improve coding standard in dbxread.c
Date: Sat, 20 Nov 2004 17:41:00 -0000	[thread overview]
Message-ID: <200411201741.iAKHf0CV001472@elgar.sibelius.xs4all.nl> (raw)
In-Reply-To: <01c4cf25$Blat.v2.2.2$44ec6860@zahav.net.il> (eliz@gnu.org)

   Date: Sat, 20 Nov 2004 19:19:42 +0200
   From: "Eli Zaretskii" <eliz@gnu.org>

   > Date: Sat, 20 Nov 2004 13:39:09 +0100 (CET)
   > From: Mark Kettenis <kettenis@gnu.org>
   > 
   > Only a tiny fraction of the code, but it's a step into the right
   > direction.

   I think changes like these are wrong:

   > -    case N_STSYM:		/* Static symbol in data seg */
   > -    case N_LCSYM:		/* Static symbol in BSS seg */
   > -    case N_ROSYM:		/* Static symbol in Read-only data seg */
   > +    case N_STSYM:		/* Static symbol in data segment.  */
   > +    case N_LCSYM:		/* Static symbol in BSS segment.  */
   > +    case N_ROSYM:		/* Static symbol in read-only data segment.  */

   The text in these comments does not constitute a full sentence, and so
   adding a period at the end is not the right change.  The right way to
   fix this, IMHO, is to lower-case the first letter of the comment, like
   this:

The coding standards say:

"Also, please write complete sentences and capitalize the first word."

Now indeed it is debatable whether the comments above are complete
sentences; there's no verb in them.  But these comments feel very
sentence-like.  Anyway, I was aiming for some consistency here.

       case N_STSYM:		/* static symbol in data segment */

   There are numerous other instances of similar changes, and IMHO they
   all are wrong.  This one looks particularly incorrect after the
   change:

   > +    case N_NOMAP:		/* No map?  (Ultrix).  */

   Some of the other changes simply reformat comments to break the line
   at a different column.  Do we have a canonical column number for that,
   and if we do, what is its value?

I always consider the GNU indentation style provided by emacs to be an
implementation of the canonical formatting style.  The canonical
column therefore would be 32.  But again, my intent here was
consistency here.  The useage was very inconsistent makeing the code
difficult to read.

Mark


  reply	other threads:[~2004-11-20 17:41 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-11-20 12:39 Mark Kettenis
2004-11-20 17:22 ` Eli Zaretskii
2004-11-20 17:41   ` Mark Kettenis [this message]
2004-11-20 18:00     ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200411201741.iAKHf0CV001472@elgar.sibelius.xs4all.nl \
    --to=mark.kettenis@xs4all.nl \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sources.redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox