This is the mail archive of the mailing list for the Cygwin project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Review required for whitespace changes?

On Wed, Jul 23, 2003 at 11:40:07AM +1000, Robert Collins wrote:
>On Wed, 2003-07-23 at 02:29, Max Bowsher wrote:
>> May I apply whitespace changes which are in keeping with the style of
>> surrounding code and the rest of setup without explicit review?
>> Are such things supposed to be ChangeLogged?
>> Example:
>> -UINT Window::IsButtonChecked (int nIDButton) const
>> +UINT
>> +Window::IsButtonChecked (int nIDButton) const
>IOW, no, no review for whitespace, but note that I'll likely be running
>astyle (a code formatter) without notice when I get some time...
>Comments that are on their own - isolated changes - should be
>changelogged though, as should non-autogenerated documentation.
>Why? Because doco changes also have motive - like code changes, and like
>code changes the motive should be recorded.

I'd submit that if the motivation for a comment change isn't obvious
then the comment change either shouldn't have been made or the comment
isn't clear.

This is what the FSF style guide has to say about it:

"There's no need to describe the full purpose of the changes or how they
work together.  If you think that a change calls for explanation, you're
probably right.  Please do explain it--but please put the explanation in
comments in the code, where people will see it whenever they see the
code.  For example, "New function" is enough for the change log when you
add a function, because there should be a comment before the function
definition to explain what it does."

So, my reading of this is that if you change a comment the ChangeLog
should say "Changed comment".  Not exactly an explanation of motive
but it does indicate that the change should be recorded contrary to
what I said.

These days the FSF advises that documentation changes should have
ChangeLog entries, too, FWIW.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]