On Thu, Jan 6, 2011 at 14:03, Matt Robinson <[email protected]> wrote:
> On Thu, Jan 6, 2011 at 11:43 AM, Daniel Pittman <[email protected]> wrote:
>> That is a very brief commit message; it would be great to have a
>> summary, and a full explanation, of the change included so that folks
>> like me can see why things were being done.
>
> I thought it was a fairly evident commit and felt more explanation
> would have been rambling. Is it not clear why the inspect app should
> have audit set to true in its reports?
Not to me, without having to go and read a whole lot more context.
More to the point...
> I'm having trouble thinking
> what I would have put in for a full explanation. Do you need
> background on what inspect does?
...it isn't clear why this change is happening, so I can't easily tell
if it is something that should be happening, or that might influence
some of the things I do know about. I would look for something like
this:
Inspect should set the audit flag because ...
So, no, not a full background on the inspect application, just at
least one-line summary of why the change was needed, to help identify
the context.
If you were generous, or there was more context, add a bigger summary
and all, but that single line makes the whole thing much easier to
identify. (Also, then I would have found the change sooner, because I
would know it was the audit flag and not, say, any of the other
arguments that had changed. :)
For what it is worth, when I write those things my audience is the
person who needs to come back and understand why a change happened -
the context about *why* I did something - in six months. Which,
usually, turns out to be me, long after I have forgotten everything I
knew today about the context.
I also find it useful when I look back over my commits before I push
to identify when I have accidentally bundled more than one change into
a single logical commit.
> I also like whitespace changes split out in general, but for such a
> short commit I didn't think it was very necessary. I figure it would
> take someone almost as long to read two commits as it would to see
> what was added in that line that changed.
You might well be right. :)
Daniel
--
✉ Daniel Pittman <[email protected]>
⌨ [email protected] (XMPP)
☎ +1 503 893 2285
♻ made with 100 percent post-consumer electrons
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
