Re: [PATCH 1/2] bisect--helper: use OPT_CMDMODE instead of OPT_BOOL

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Chris,

On Wed, 4 May 2016, Christian Couder wrote:

> On Wed, May 4, 2016 at 4:56 PM, Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
> >
> > On Wed, 4 May 2016, Christian Couder wrote:
> >
> >> My intent was to try to show that there is some important value to make
> >> the subject close to the "low level" thing the patch actually does.
> >
> > I disagree. The place to describe low-level details that are not
> > immediately obvious from the patch is the commit message. Way, way down on
> > the bottom.
> >
> > The commit message should start with a subject that gives me a good clue
> > why this is a good change. A low-level detail won't do that very often.
> 
> Let me give you another example.
> 
> You want to do X and you discover that you need to tweek knob foo to
> do X, and also that tweeking knob foo is a good thing to do in general
> (for example it could be a refactoring that save some lines of code).
> So you create a patch that tweeks knob foo and call it "prepare to do
> X", and then send it so that it can be merged. Then you want to do X
> but you are interupted by an event, for example you boss tells you to
> do Y instead of X right away because of an urgent business need. So
> you do Y instead of X. And then sometimes later (it could be weeks,
> months or years later), when you are finished with Y, you now want to
> go back to your previous work on X. At that time you discover that you
> need to tweek knob bar (it could be another refactoring of another
> part of the code), but you forgot about your previous patch that
> tweeked knob foo, so you call your new patch that tweeks knob bar
> "prepare to do X". As your previous patch that was also called
> "prepare to do X" has already been integrated, you now have too
> patches that do different things that are called the same, and you can
> easily mistake the first one for the second one.
> 
> If you had just called your first patch "tweek knob foo" and the
> second patch "tweek knob bar", it would be much clearer which patch
> does what.

And then, six months later, when *you* are stuck in a plane and bisect a
problem down to my "tweak knob foo", you curse at me, asking fellow
passengers "what was Dscho thinking? 'foo'? What the hell is 'foo'
supposed to mean? And what was the *intention* of this patch? There is a
bug! And now I cannot fix it properly because I do not understand what the
patch was intended to do!".

Do not believe even one second this scenario s foreign to *me*. Even with
some of git.git's commits whose messages were so in love with low-level
details, essentially repeating what the diff already told me, that they
forgot to include the context, the big picture, leaving me puzzled and
frustrated.

And my fellow passengers, too.

This puzzlement and frustration is unnecessary. I typically follow Peff's
examples. They typically do a good job of telling me right away what the
big picture is, even if they are in the middle of a patch series, not
worrying too much whether adjacent commits say related things, without
forgetting low-level details. The crucial point is that they start out by
assuming little familiarity of the reader with the context.

In short: you can have both. The commit message *can* be informative, and
*still* contain low-level details. You simply start with the former and
ease the reader into the latter, instead of skipping the first part, is
all.

Ciao,
Johannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]