Re: Patches with independent changes

[Eli Zaretskii]

> Date: Sat, 25 Jan 2014 12:58:09 -0800
> From: Paul Eggert <address@hidden>
> CC: address@hidden
> 
> Eli Zaretskii wrote:
> > otherwise the build would have failed when compiler warnings are
>  > turned on and used as errors.
> 
> That would have been odd, as other static variables in Emacs are 
> declared and set but never used (e.g., category_table_version) and 
> evidently these builds are not failing.  It's certainly OK to clean up 
> the glitches, but such cleanups are independent changes.

They are not independent.  A variable that becomes unused as result of
a change should be removed as part of that change.

> > Then any number of patches can be installed in a single commit,
> > because they are all "related" -- after all, they are all about Emacs.
> 
> No, that's too broad.  Patches should contain changes that are more 
> closely-related than that.

That's the gist of our disagreement: how close that relation should
be.  Using vague terminology, such as "related", doesn't solve it.

>  > you cannot chmod a file that is open ... the chmod call will fail.
> 
> Where is this documented?  Does the problem occur if any process has the 
> file open, or only if the current process has it open?  What is errno 
> after the failure?  This problem does not occur on POSIXish platforms; 
> if it happens under Microsoft Windows the incompatibility should be 
> documented (in Gnulib, if the problem is generic to GNU applications).

Then document it, please.  Gnulib documentation is not of interest
here.

>  > the fact that the problem could have been solved in more than
>  > one way doesn't mean there are multiple changes involved.
> 
> True, but if the problem could have been solved in a simpler way that 
> involved fewer changes, then multiple changes were most likely present.

It's not simpler.

> Come to think of it, why is that chmod needed at all in WINDOWSNT?  The 
> file is already readable and writeable, so as I understand it chmod 644 
> is therefore a no-op on that platform.  If so, the attached patch would 
> have been simpler yet, no?

Removal of this code _could_ have constituted an additional change
unrelated to the original bug.

In any case, we don't want to consider changes that aren't bugfixes at
this time.  The change I made restored code that was there in the
first place.  Any improvements can be considered at a later date.