[PATCH] parser: Use a series even if the version differs

[Stephen Finucane]

On Mon, 2017-06-26 at 21:01 +1000, Daniel Axtens wrote:
> Hi Stephen,
> 
> > Currently, once we've found a series for a new submission, we check to
> > ensure that the version of that series matches that of the submission,
> > and we discard the series if not. The original intention for this was
> > given in commit 'c21b30525', where this was added:
> > 
> >   There are some things you probably shouldn't do on public mailing
> >   lists, but which people do anyway.
> > 
> >   - [PATCH 1/2] test: Add some lorem ipsum
> >     - [PATCH 2/2] test: Convert to Markdown
> >       - [PATCH v2 1/2] test: Add some lorem ipsum
> >         - [PATCH v2 2/2] test: Convert to Markdown
> > 
> >   We should correctly parse these...
> > 
> > This is unnecessary for two reasons:
> > 
> > - If the series is using proper references, then the mechanism that
> >   we use to prevent this issue with unversioned follow ups (also added
> >   in that patch) will work here: namely, we don't add submissions if an
> >   submission with a given number already exists in that series.
> > - If the series is not using references, we already have a check for
> >   version.
> > 
> > Seeing as this check is actually causing issues for legitimate typos, we
> > should just remove this check. Do so, adding a check to ensure we don't
> > regress.
> 
> I'd really really really prefer people used git-send-email and/or
> whatever the mercurial equivalent is. The tools aren't *that* difficult
> to use correctly. But having said that, I'm not necessarily opposed to
> this patch. Not super keen, but not super opposed.

In general, I would agree. However, it is surprisingly easy to make this
mistake yourself. If I weren't aware of the 'git-send-email --reroll-count'
option, I would probably default to encoding the version in the subject line of
the cover letter. We actually assume that some people will do this in our
version check [1]. Coupled with the fact that this check doesn't make any of
our existing cases less robust (see above), I think this is a prudent move and
a real world demonstration of the Robustness Principle [2]. FWIW, the
Robustness Principle was the eventual reason for not merging patches to
optionally restrict the parser to 'git-send-email'-formatted patches.

> 1 comment below.
> 
> Regards,
> Daniel
> 
> > 
> > Signed-off-by: Stephen Finucane <stephen at that.guru>
> > Closes-bug: #105
> > ---
> >  patchwork/parser.py                                |   1 -
> >  .../tests/series/base-different-versions.mbox      | 908
> > +++++++++++++++++++++
> >  patchwork/tests/test_series.py                     |  18 +
> >  3 files changed, 926 insertions(+), 1 deletion(-)
> >  create mode 100644 patchwork/tests/series/base-different-versions.mbox
> > 
> > diff --git a/patchwork/parser.py b/patchwork/parser.py
> > index 794a5ea..c43fe76 100644
> > --- a/patchwork/parser.py
> > +++ b/patchwork/parser.py
> > @@ -911,7 +911,6 @@ def parse_mail(mail, list_id=None):
> >          #    * the version doesn't match
> 
> You'll need to remove this comment line too.

Good call. I can do this when I apply the patch.

Stephen

[1] https://github.com/getpatchwork/patchwork/blob/v2.0.0-rc4/patchwork/parser.
py#L407-L409
[2] https://en.wikipedia.org/wiki/Robustness_principle