Hi,
On 16/10/18 07:37, Leonardo Brondani Schenkel wrote:
On 15 Oct 2018, at 11:18 pm, Chris Jones <jonesc at hep.phy.cam.ac.uk>
wrote:
On 15 Oct 2018, at 10:34 pm, Leonardo Brondani Schenkel <lbschenkel
at macports.org> wrote:
I'm a committer, and if I'm doing a trivial bump of an openmaintainer
port I'll push it directly.
Depends entirely on what you consider trivial. If you consider a
version update a trivial bump then I disagree. This in my opinion is
something that should also be submitted as a PR so the port maintainer
always gets a chance to review it. If the port is open maintainer and
they do not respond within the 72 hour timeout, it can then be merged
without their comment, but not before. For a committer to directly
commit an update to a port that isn’t trivial and they do not maintain
is in my humble opinion an abuse of the member role. Members should
not bypass the PR review stage just because they can.
Like it or not, that's the definition of 'openmaintainer': for simple
version bumps, no maintainer approval is necessary.
Please point me to where this is documented ? i.e. where is it stated
that openmaintainer allows revision changes. This appears to be one of
the issues here since not everyone, myself included, agrees with you.
The only statement I have found is
"If a port's maintainer contains the address
<[email protected]>, this means that the author allows minor
updates to the port without contacting him first. But permission should
still be sought for major changes."
which does not mention this point. It leaves the interpretation of minor
and major to the reader.
Chris
Since the maintainer
is the one explicitly opting in to such policy, I don't see any issue
with respecting their wish. I deliberately added 'openmaintainer' to all
the ports I maintain so others are free to do version bumps without
going through me first, reducing friction (but not other changes, see
below) >
Ports that tend to break on any minor version bump, or are problematic
in other ways, should probably not be marked as 'openmaintainer' -- and
I believe they aren't.
If I'm opening a GitHub PR for an openmaintainer port this means that
for some reason I want the maintainer's opinion/review before it gets
merged.
In my opinion the ‘rules’ for when this happens should not be
subjective, as they currently seem to be. There needs to be a clear
policy on what is and is not trivial and thus what does and does not
require PR review.
I agree that it looks like different people have different
interpretations for the scope of 'openmaintainer' (apart from simple
version bumps being allowed, which is the definition that is explicitly
written in the documentation).
My own interpretation is that any change that is not a simple version
bump, a simple revision bump due to a dependency being upgraded, or
fixing a livecheck or a typo, do not fall within the scope of
'openmaintainer' and should be cleared with the maintainer first.
My belief is that in general nobody else should merge the PRs besides
the submitter of the PR and/or the maintainer.
Disagree. Any member with commit rights should be able merge PRs. As
long as the procedures have been followed, so either the maintainer
has oked the changes, or the 72 hour timeout has expired and another
reviewer has agreed with the changes, it does not really matter who
finally clicks merge.
Naturally any member can merge PRs by virtue of having the rights to do
so, but my point was that when a PR is opened against a maintained port,
the recipient for that PR is the maintainer although it can be
additionally reviewed/commented by anybody else (I suspect most PRs
don't). Barring maintainer timeouts and other special circumstances,
it's the maintainer that ultimately decides if that PR should be merged.
At that point, they can signal it by approving the PR -- and depending
on who has commit rights they can merge it themselves, or ask the
submitter to merge, or ask any non-participant member to merge.
( Its actually quite common practise in many circles for submitters of
a PR to require third party approval, and for someone else to merge
it, and for the submitter to simply do it themselves to be discouraged. )
In the scenarios I have in mind and I participated in, the submitter
merges if they're asked to by the maintainer or when the PR is approved
without requests for changes or other concerns. Sometimes I submit PRs
to other ports maintained by non-members, and I'm the one who has commit
rights.
They can merge at any time of their choosing in case they want to
proceed, and there is no need to second guess their intentions.
Only the maintainer of a port should be free to merge before the 72
hour timeout, if they have the rights to do so. If they do not have
commit rights, then can signify they have agreed to the changes at
which point any member could merge.
I think I wasn't clear on that point. What I meant is that a PR is
either on a 'waiting for review/reviewing' phase, or an 'approved/ready
to be merged' phase, and once the PR is 'approved' then anybody can
merge. If it's on the 'review' phase, it's up to the participants to
transition that PR into 'approved' and merge (or ask for it to be
merged). I see very little room here for a non-participant third-party
to second guess a participant's intention and prematurely merge a PR --
and if this seems to be warranted, simply asking first will eliminate
the vast majority of misunderstandings.
// Leonardo.
