Re: Patch: grep ^func

[Michael Snyder <msnyder at cygnus dot com>]

Eli Zaretskii wrote:
> 
> > Date: Fri, 26 Jan 2001 09:15:29 -0800
> > From: Michael Snyder <msnyder@redhat.com>
> >
> > Eli Zaretskii wrote:
> >
> > > In general, I think gratuitous changes in whitespace should be
> > > avoided, since they get in the way when you need to figure out what
> > > _real_ changes in the code happened since the last time you looked.
> >
> > Strongly disagree.  Some of the code in GDB is a real mess, and
> > I am devoted to the goal that it will look better (as time
> > becomes available.
> 
> I don't see any disagreement, just a terminology problem ;-).  Fixing
> code that looks like a car crash is not ``gratuitous changes'' in my
> book.
> 
> However, removing a newline from a comment in a case like the one in
> point, where the newline was put on purpose is IMHO nowhere near
> fixing messy code.

I didn't see that -- by whitespace, I thought you meant the newlines
in the function prototypes -- a change that I agree with.

Changing newlines in comments is also potentially a good thing
in some cases -- sometimes they're bad just from carelessness etc.
Of course if they were carefully arranged by the author for some
good reason, that is different -- but then the author always has
the opportunity to speak up and object (assuming he's on this list).

So I would rather say that whitespace changes _can_ be bad, if
they violate the author's intentions, rather than that they are
generally bad.

> > Clean-up patches should be made separately from bug-fix
> > patches, but they should NOT be avoided.  CVS allows you
> > to separate them, Eli.  Avoiding doing clean-up because it
> > makes using diff more difficult would be a bad bad thing,
> > IMO.
> 
> Please note: I was talking specifically about whitespace, and then
> only about gratuitous changes.  This explicitly excludes changes meant
> to bring GDB's code to GNU standards.
> 
> Such gratuitous whitespace changes make it hard to review patches
> posted here for comments.  Instead of simply reading the patch, one
> needs to actually patch the files, then look at the old and new
> versions with some tool such as Ediff.  IMHO, making the reviewers'
> lives more complicated for no good reason is nor a good idea, if we
> want the approval process to be efficient.

But this patch was exclusively a whitespace / formatting change.
I agree that mixing whitespace changes in with bug fixes is bad, 
but submitting them separately is not.