Re: contributing to Emacs

[Eli Zaretskii]

> From: Konstantin Kharlamov <hi-angel@yandex.ru>
> Cc: luangruo@yahoo.com, arne_bab@web.de, ams@gnu.org, emacs-devel@gnu.org
> Date: Sat, 24 Jun 2023 15:36:20 +0300
> 
> On Sat, 2023-06-24 at 10:43 +0300, Eli Zaretskii wrote:
> > 
> > What most people do instead is they provide a series where each patch
> > is a step towards the solution.  First, a patch with some refactoring,
> > then another patch with the first aspect of the solution, another
> > patch with the second aspect, etc.  Such series make no sense as a
> > series, because the patches are not really independent; instead, they
> > are _incremental_.  For example, it usually makes no sense to do the
> > refactoring if we aren't installing the changes which need it.
> 
> I might be misunderstanding something, but as someone who regularly posts such
> "incremental changes" at my work, I tend to disagree that such refactoring 
> will
> not be needed if the final change not applied.

You might disagree, but such considerations and decisions are the
prerogative of the project maintainers, not of the contributors.  In
Emacs, we don't like code churn unless it has a purpose, and
refactoring by itself doesn't justify the downsides of making the code
less familiar to those who read and audit it very frequently, and need
to be able to find the relevant parts as fast as possible.  Keep in
mind that this is an old project with code written by excellent
programmers (I exclude myself from that group, obviously), and the
code is generally in very good shape.  Thus, refactoring is not really
an urgent need, like it might be in an average project out there.

Again, you might disagree, but this is not your call.  My point was
precisely that since it is not the call of the contributors, they
cannot be expected to make those decisions on our behalf, and are
therefore better off not dividing the patches into several individual
ones.

> > Moreover, this technique frequently leads to multiple patches touching
> > the same places several times, so when you review the first patch, you
> > are looking at code that will be modified later, and risk providing
> > comments that are irrelevant, because a later patch in the series
> > rewrites that code anyway, perhaps exactly in a way that you want to
> > tell the contributor to use.
> 
> Actually, this is exactly how review works. It *does not* matter if later code
> rewrote the same place again. If you found a problem in *current* 
> commit/patch,
> that means that exactly *this* commit/patch needs to be fixed.

I think you misunderstood what I tried to explain, because you are
actually saying that wasting effort on reviewing of and commenting on
code that will be changed or reverted by a further commit is a Good
Thing.

But even if you did understand my point, are you really going to tell
me how to review patches?  Don't you think it's my decision, and not
yours?  In the project of which you are the maintainer, you can define
the procedures and the preferences as you see fit (and I will follow
if I need to contribute, as I do, for example, with GDB, where they do
want patch series), but you cannot force me use the procedures that
you find convenient and natural in a project where I need to review so
many patches each day.

Bottom line: I explained the rationale so that people could understand
the preferences better, and in particular realize that they aren't
arbitrary.  I hope that better understanding will make it easier to
respect the preferences, even if some don't agree with them.