Re: contributing to Emacs

[Sean Whitton]

Hello,

On Sat 24 Jun 2023 at 10:43AM +03, Eli Zaretskii wrote:

> The problem with that preference is that most contributors
> misunderstand how to arrange the series correctly, and end up sending
> series which require extra work, because the same places were modified
> several times.  On top of that, the series doesn't really allow
> applying only some of the patches, which is the main raison d'être of
> the technique.
>
> To correctly subdivide a large patch into a series, one must identify
> changes in the changeset that are (a) independent, i.e., can be
> applied without all the rest, and in any order; and (b) changes which
> _make_sense_ as separate changes, so the project might consider
> applying just them, even though the rest will be rejected.
>
> 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.
> 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.  Basically, the only sane method of
> reviewing such "series" is to apply the entire series, then produce
> the unified diffs from all of them, and than review those diffs
> instead of the patch series.
>
> Since the correct way of breaking patches into individual ones is hard
> to explain, and requires good knowledge of the project's conventions
> and practices to determine how to break the large patch, I find it
> easier to ask contributors to not bother with series and submit a
> single large patch.  IME, it's TRT in the majority of cases anyway,
> especially with contributions that provide some minor change or fix,
> and it definitely makes patch review much more convenient and easy
> than the opposite preference.

This all makes sense.  Separating changes usefully is a skill that
people have to acquire.  And it's reasonable to conclude from experience
that enough people get it wrong that it is better to ask people not to
do it at all.

I do disagree that the main reason for separating them is to allow
applying only some of them.  I think making review easier, and improving
the level of detail in the VCS history, are more singificant reasons.

-- 
Sean Whitton