[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 0/3] XSA-343 followup patches



Julien Grall writes ("Re: [PATCH v6 0/3] XSA-343 followup patches"):
> On 18/11/2020 08:22, Jan Beulich wrote:
> > Without a clear outline of what would break with the present logic,
> > I had previously indicated I'm not convinced of the change. This
> > isn't a strong objection, no, but I still wouldn't want to see my
> > name associated with it in such a case.
> 
> I was under the impression that the committer job is mostly mechanical 
> (i.e. collecting tags and applying patches). There are no rules in 
> MAINTAINERS that mention committers can decide what gets committed as 
> long as maintainers approved it and there are no strong objections.

I think in practice committers need to exercise quite a bit of
judgement.  I often find myself deciding on what seem to be edge cases
of the rules.  I also sometimes, with something which has the formally
needed acks, but which seems like it might be controversial or
awkward, decide to just make an extra double-check.  I sometimes even
commit something when maybe the formal rules wouldn't permit it, when
I'm confident that the relevant maintainer(s) etc. wouldn't object -
an example being when something is badly broken and this is allegedly
the fix.

However, I don't agree that a committer should omit committing
something which is acked, and is part of a series which they are
committing, simply because they don't see the need for the patch.

If a committer who is also a maintainer has a firm objection, they
should state that objection as a formal NAK.  If a formal NAK is not
warranted, a committer should not engage in passive obstruction.

Also, a committer should not "silently" commit only part of a series.


In summary, committing something is not a declaration by the committer
that they approve of the patch on a technical level.

It is a declaration that (in the committer's view) the patch is
properly acked/approved, and that therefore according to our shared
understanding of the community's processes, it ought to go in.

That might even occur if the committer has an outstanding technical
objection.  I have certainly committed things that I didn't really
like very much, on the grounds that they had enough acks and that I
didn't feel I wanted to give it a formal NAK.


If it would help a committer feel more comfortable to be explicit
about this, there are a number of approaches available: the committer
could commit the patch but also reply to it on-list restating their
objection but saying that they are committing it despite the
objection, because of others' acks.

If it is felt desirable to record this information in the repository,
one could write something like
   Not-Acked-by: Ian Jackson <iwj@xxxxxxxxxxxxxx>
(which I think is not the same as Nacked-by) or even add a note in
the commit message like
  [ I am committing this, even though I don't think it is
    necessary, because it has the requisite acks.  I do not
    think it warrants a nack.  -iwj ]

> This is a matter of perspective. It helps to confirm with the
> contributor that it was fine to merge only part of the series
> (multiple pair of eyes is always better than one...) or mentioning
> any clash/reworked.

I think it is especially important to send an email about things being
committed when what has happened might be surprising.  For example, if
only part of a series is committed.

Thanks,
Ian.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.