linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Randy Dunlap <rddunlap@osdl.org>
To: <joe.korty@ccur.com>
Cc: <colpatch@us.ibm.com>, <pj@sgi.com>, <akpm@osdl.org>,
	<paulus@samba.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] bitmap parsing/printing routines, version 4
Date: Mon, 19 Jan 2004 21:41:38 -0800 (PST)	[thread overview]
Message-ID: <1252.4.5.45.142.1074577298.squirrel@www.osdl.org> (raw)
In-Reply-To: <20040120035756.GA15703@tsunami.ccur.com>

> On Mon, Jan 19, 2004 at 01:17:26PM -0800, Matthew Dobson wrote:
>> Joe,
>>
>> Additions:
>> 4) A whole bunch of comments.  Are these all correct?
>>
>> None of the things in my patch (with the possible exception of #3)
>> change the functionality of the code, which looks great.
>>
>> Andrew, I agree with Paul's "thumbs-up" of Joe's patch.  My patch is
>> solely meant to increase the readability of the bitmap_parse function.
>>
>> Cheers!
>>
>> -Matt
>
> Indeed you are correct, the final (totaldigits == 1) test can be
> removed. Good catch.
>
> However, IMHO you added too many comments.  Unlike Andrew, I do believe
> one can have too many comments.  Comments become 'too many' when they
> dilute to the point that the code can no longer be clearly read.

Sure, some code can have too many comments, but most of Linux isn't
approaching that level.

> If you reduce the comments to just those that say something not easily
> deduced from the code, then they would be acceptable to me, and would
> make a useful addition IMO.  That would be all but three, or perhaps
> four, of them.
>
> Andrew, if you do like the fully commented version, then please remove
> my name from the comment in the patch.  The dilute style of coding is
> not one I wish to have my name associated with.

~Randy




  parent reply	other threads:[~2004-01-20  5:41 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-01-07 16:56 seperator error in __mask_snprintf_len Joe Korty
2004-01-07 19:32 ` Andrew Morton
2004-01-08 13:11   ` Paul Jackson
2004-01-08 22:50     ` Paul Mackerras
2004-01-08 22:59       ` Joe Korty
2004-01-09  0:07         ` Paul Mackerras
2004-01-09  1:11           ` Paul Jackson
2004-01-14 23:03           ` Paul Jackson
2004-01-15  0:27             ` Joe Korty
2004-01-15  0:37               ` Paul Jackson
2004-01-15  4:40               ` Paul Jackson
2004-01-15 16:15                 ` Andrew Morton
2004-01-15 18:15                   ` Joe Korty
2004-01-16  0:17                     ` Paul Jackson
2004-01-16  0:48                       ` Joe Korty
2004-01-16  1:48                         ` Paul Jackson
2004-01-16 23:29                       ` Matthew Dobson
2004-01-17  6:36                         ` [PATCH] bitmap parsing routines, version 3 Joe Korty
2004-01-17 10:08                           ` Paul Jackson
     [not found]                             ` <20040117145545.GA16318@tsunami.ccur.com>
2004-01-17 15:36                               ` Joe Korty
2004-01-17 23:33                                 ` Paul Jackson
2004-01-18  5:52                                   ` William Lee Irwin III
2004-01-18  7:03                                     ` Paul Jackson
2004-01-17 18:39                           ` [PATCH] bitmap parsing/printing routines, version 4 Joe Korty
2004-01-17 23:36                             ` Paul Jackson
2004-01-19 21:17                             ` Matthew Dobson
2004-01-20  0:17                               ` Paul Jackson
2004-01-20  3:57                               ` Joe Korty
2004-01-20  4:15                                 ` Paul Jackson
2004-01-20  5:41                                 ` Randy Dunlap [this message]
2004-01-20  7:03                                 ` Matthew Dobson
2004-01-20 15:36                                   ` Joe Korty
2004-01-20 17:06                                     ` Matthew Dobson
2004-01-17  9:12                         ` seperator error in __mask_snprintf_len Paul Jackson
2004-01-16  5:14                     ` Paul Jackson
2004-01-16  5:26                       ` Andrew Morton
2004-01-16  5:52                         ` William Lee Irwin III
2004-01-16 14:23                       ` Joe Korty
2004-01-17 10:07                         ` Paul Jackson
2004-01-15 22:53                   ` Paul Jackson
2004-01-16  1:06                     ` Andrew Morton
2004-01-16  2:54                       ` Paul Jackson
2004-01-09 14:28         ` Paul Jackson
2004-01-09 14:46       ` Paul Jackson
2004-01-09 15:14         ` Andreas Schwab
2004-01-09 15:25           ` Christoph Hellwig
2004-01-09 17:23             ` Paul Jackson
2004-01-12  0:09               ` Joe Korty
2004-01-12 21:41                 ` Paul Jackson
2004-01-12 22:00                   ` Joe Korty
2004-01-12 22:28                     ` Paul Jackson
2004-01-12 22:39                       ` Joe Korty
2004-01-09 14:57       ` Paul Jackson
2004-01-08  1:06 ` Paul Jackson
2004-01-08  3:32   ` Joe Korty
2004-01-08 10:39     ` Paul Jackson

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=1252.4.5.45.142.1074577298.squirrel@www.osdl.org \
    --to=rddunlap@osdl.org \
    --cc=akpm@osdl.org \
    --cc=colpatch@us.ibm.com \
    --cc=joe.korty@ccur.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulus@samba.org \
    --cc=pj@sgi.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;
as well as URLs for NNTP newsgroup(s).