All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, Peter Zijlstra <peterz@infradead.org>,
	Paul McKenney <paulmck@linux.vnet.ibm.com>,
	John Stultz <john.stultz@linaro.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Frederic Weisbecker <frederic@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>,
	Andy Lutomirski <luto@kernel.org>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Will Deacon <will.deacon@arm.com>,
	Mark Brown <broonie@kernel.org>,
	Dan Williams <dan.j.williams@intel.com>
Subject: Re: [patch 2/2] Documentation/process: Add tip tree handbook
Date: Thu, 8 Nov 2018 08:14:13 +0100	[thread overview]
Message-ID: <20181108071413.GA25195@gmail.com> (raw)
In-Reply-To: <20181108070547.GA20032@gmail.com>


* Ingo Molnar <mingo@kernel.org> wrote:

> With tail comments the code looks like this:
> 
> 	res = dostuff(); /* We explain something here. */
> 
> 	seed = 1; /* Another explanation. */
> 
> 	mod_timer(&our_object->our_timer, jiffies + OUR_INTERVAL); /* We like to talk */
> 
> 	res = check_stuff(our_object); /* We explain something here. */
> 	if (res)
> 		return -EINVAL;
> 
> 	interval = nontrivial_calculation(); /* Another explanation. */
> 
> 	mod_timer(&our_object->our_timer, jiffies + interval); /* This doesn't race, because. */
> 
> ... while with freestanding comments it's:
> 
> 	/* We explain something here: */
> 	res = check_stuff(our_object);
> 	if (res)
> 		return -EINVAL;
> 
> 	/* Another explanation: */
> 	interval = nontrivial_calculation();
> 
> 	/* This doesn't race with init_our_stuff(), because: */
> 	mod_timer(&our_object->our_timer, jiffies + interval);
> 
> This comment placement style has several advantages:
> 
>   - Comments precede actual code - while in tail comments it's exactly
>     the wrong way around.
> 
>   - We don't create over-long lines nor artificially short tail comments 
>     just because we were trying to stay within the col80 limit with the 
>     "this doesn't race" comment. The full-line comment was able to 
>     explain that the race is with init_our_stuff().
> 
>   - Freestanding oneliner comments are much better aligned as well: note 
>     how ever comment starts at the exact same column, making it very easy 
>     to read (or not to read) these comments.
> 
>   - In short: predictable visual parsing rules and proper semantic 
>     ordering of information is good, random joining of typographical 
>     elements just to create marginally more compact source code is bad.
> 
>   - Just *look* at the tail comments example: it's a visually diffuse, 
>     jumble of statements and misaligned comments with good structure.

Forgot to mention the role of punctuation:

    - Also note how punctuation actually *helps* the integration of 
      comments and code. The ":" will direct attention to the code that 
      follows, making it clear that the sentence explains the next 
      statement. There are exceptions for when different type of 
      punctuation is a better choice, for example:

	/* TODO: convert the code to our newest calc API ASAP. */
	interval = nontrivial_calculation();

      Here we don't explain the statement but document a TODO entry. 

      Also, it's frequent practice to use multi-line comments to explain 
      the next section of code, with a particular note about the first 
      statement. Proper punctuation helps there too:

	/*
	 * Establish the timeout interval and use it to set up
	 * the timer of our object. The object is going to be
	 * freed when the last reference is released.
	 *
	 * Note, the initial calculation is non-trivial, because
	 * our timeout rules have complexity A), B) and C):
	 */
	interval = nontrivial_calculation();

      Note how part of the multi-line comment describes overall 
      principles of the code to follow, while the last sentence 
      describes a noteworthy aspect of the next C statement.

Thanks,

	Ingo

  reply	other threads:[~2018-11-08  7:14 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-07 17:10 [patch 0/2] Documentation/process: Add subsystem/tree handbook Thomas Gleixner
2018-11-07 17:10 ` [patch 1/2] Documentation/process: Add maintainer handbooks section Thomas Gleixner
2018-11-07 17:10 ` [patch 2/2] Documentation/process: Add tip tree handbook Thomas Gleixner
2018-11-07 17:44   ` Thomas Gleixner
2018-11-07 19:57     ` Paul E. McKenney
2018-11-07 19:38   ` Paul E. McKenney
2018-11-08  7:05   ` Ingo Molnar
2018-11-08  7:14     ` Ingo Molnar [this message]
2018-11-08  7:19   ` Ingo Molnar
2018-11-08  7:30   ` Ingo Molnar
2018-11-08  7:40   ` Ingo Molnar
2018-11-08  9:12     ` Peter Zijlstra
2018-11-08 11:05       ` Greg Kroah-Hartman
2018-11-08 17:19       ` Dan Williams
2018-11-08 17:24         ` Borislav Petkov
2018-11-08 17:40         ` Paul E. McKenney
2018-11-08 19:58           ` Thomas Gleixner
2018-11-08 20:05             ` Paul E. McKenney
2018-11-08 21:06             ` Greg KH
2018-11-08 21:08               ` Greg KH
2018-11-08 22:38               ` Thomas Gleixner
2018-11-08 20:14         ` Theodore Y. Ts'o
2018-11-08 20:22           ` Thomas Gleixner
2018-11-08 21:04           ` Greg KH
2018-11-08 22:19             ` Theodore Y. Ts'o
2018-11-08 22:33               ` Greg KH
2018-11-08 22:56           ` Dan Williams
2018-11-08  7:46   ` Ingo Molnar
2018-11-08  8:04   ` Ingo Molnar
2018-11-08  8:13   ` Ingo Molnar
2018-11-08  8:18   ` Ingo Molnar
2018-11-08  8:30   ` Ingo Molnar
2018-11-07 19:48 ` [patch 0/2] Documentation/process: Add subsystem/tree handbook Jonathan Corbet
2018-11-07 19:58   ` Dan Williams
2018-11-07 20:51     ` Thomas Gleixner
2018-11-08 14:49       ` Jonathan Corbet
2018-11-08 15:05         ` Peter Zijlstra
2018-11-08 15:19           ` Jonathan Corbet
2018-11-08 16:05             ` Paul E. McKenney
2018-11-08 16:21           ` Theodore Y. Ts'o
2018-11-08 16:32             ` Mark Brown
2018-11-08 17:32               ` Dan Williams
2018-11-13 23:15                 ` Mark Brown
2018-11-08 16:33             ` Jonathan Corbet
2018-11-08 19:46               ` Theodore Y. Ts'o
2018-11-08 15:49         ` Thomas Gleixner
2020-12-17 15:05           ` Borislav Petkov
2020-12-17 17:53             ` Jonathan Corbet
2020-12-17 17:58               ` Borislav Petkov
2018-11-12  5:52         ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2021-09-13 15:39 [PATCH 0/2] doc: Add tip maintainer's handbook Borislav Petkov
2021-09-13 15:39 ` [PATCH 2/2] Documentation/process: Add tip tree handbook Borislav Petkov

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=20181108071413.GA25195@gmail.com \
    --to=mingo@kernel.org \
    --cc=acme@redhat.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=broonie@kernel.org \
    --cc=corbet@lwn.net \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=frederic@kernel.org \
    --cc=john.stultz@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=will.deacon@arm.com \
    --cc=x86@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.