Merging pull requests before 72 hours

[Blair Zajac]

We could add a rule that should help a bit that openmaintainer only lets people do minor version bumps, e.g. X.Y to X.(Y+1) and X.Y.Z to X.Y.(Z+1). This doesn’t solve the Lua 5.2 to 5.3 one, but it would prevent the Python 2.7 to 3.7.

Blair

> On Oct 14, 2018, at 3:06 PM, Mojca Miklavec <mojca at macports.org> wrote:
> 
> Hi,
> 
> This conversation comes from
>    https://github.com/macports/macports-ports/pull/2785
> and I wanted to bring it up to the mailing list.
> 
> In summary: Ryan objected merging openmaintainer pull requests prematurely.
> I'm with Ryan on this one.
> 
> On Fri, 12 Oct 2018 at 15:04, Perry E. Metzger wrote:
>> On Fri, 12 Oct 2018 at 07:39, Ryan Schmidt wrote:
>>> On Thu, 11 Oct 2018 at 18:01, Perry E. Metzger wrote:
>>>> 
>>>> I had little confidence that he would know it happened,
> 
> If this is ever the case, we would need to change our system. But I
> agree that the maintainer must have seen this. If he didn't, we need
> to let him know about the problem (there are some who disabled all
> email traffic and then never get any notifications; those maintainers
> should fix their preferences to include email for cases when they are
> mentioned).
> 
>>>> plus it's listed as
>>>> openmaintainer, and, like it or not, your updates are almost always perfect,
>>>> there's rarely any dissent against them, so you get treated differently.
>>> 
>>> Why? I assigned the PR to him, and macportsbot also notified him.
>>> 
>>>> plus it's listed as openmaintainer,
>>> 
>>> Yes but in the past some maintainers have objected even to simple version
>>> updates being performed on their openmaintainer ports without their consent.
>>> Developers who are not the maintainer might not be aware all the implications
>>> that updating the version of one port might have on other ports for example.
> 
> Precisely. What may seem like a totally trivial and straight update to
> you, consisting of nothing else but version bump and checksum changes,
> may introduce dramatic changes or incompatibilities.
> 
> (Oh, look, python 2.7 has now been update to python 3.x, let's update
> it. Oh, see, a new version of poppler just came out. Let's update just
> this one; the travis job succeeds, that's sufficient right?)
> 
> Also, many people hardly test anything when submitting a PR. I need to
> confess that is often true for me even for commits that circumvent the
> PR. If a version of a port I trust changes, I would often only check
> whether the build succeeds, or if the test suite passes in case there
> is one (which is most often not the case). The cases when I go to the
> trouble of verifying the consequences on all the dependent ports
> beyond whether or not they build are very very very rare.
> 
> A while ago we had something that looked like a completely innocent
> update of Lua (5.2 to 5.3) which literally broke EVERY SINGLE
> dependency, most of them beyond repair. I knew this would happen, but
> someone simply went forward and committed the update without asking
> anyone for opinion. Circumventing the maintainer might have similar
> consequences.
> 
>>> And this PR was more than just updating the version: I also reformatted the
>>> entire port's whitespace. While I stand by those changes, reformatting someone
>>> else's port is not something we should probably do without asking permission first;
>>> this PR was how I was asking Toby's permission, and by merging the
>>> PR before he had a chance to look at it, he didn't get the opportunity to give it.
>>> 
>>>> and, like it or not, your updates are almost always perfect, there's rarely any
>>>> dissent against them, so you get treated differently.
>>> 
>>> If I want to commit an update, I'll do that. If I go to the effort of filing a PR,
>>> it's because I want to give the maintainer of the port the opportunity to be aware
>>> of the proposed change and to voice any objections before it gets committed.
>>> I appreciate that you reviewed my changes and found them suitable, but if a
>>> port has a maintainer, we should really give them the opportunity to fulfill that role.
> 
> Agreed.
> 
>> Then they should remove openmaintainer from their port.
> 
> While I'm not entirely sure about the exact difference between
> maintainer and non-maintainer, I disagree.
> 
>> openmaintainer means
>> that you can perform simple version updates without explicit approval.
> 
> Simple changes. What looks like a simple version update might not be
> that (let's update python 2.7 to python 3.7, just version change and
> checksum updates, simple ...)
> 
>> If they don't want that, they can just commit a change removing it.
>> 
>> We are in a situation where most, if not the overwhelming bulk, of our ports with
>> maintainers are de facto abandoned.
> 
> Which means that we should address that particular problem (maybe more
> aggressively than we do now) rather than ignore maintainer field. If
> you commit within a few hours, we don't even know whether a port is
> abandoned or not. Before PRs were a thing, we would always append
> "maintainer timeout" to commit messages. This happened to be super
> useful. When I would want to update a port, I would run svn/git log
> and if all the commit messaged said "maintainer timeout" a couple of
> times, this would send a very strong signal that I should probably
> file a port abandoned ticket.
> 
> Maybe we should reintroduce this and always when there's a 72-hour
> maintainer timeout:
> - change the commit message
> - check whether the person is otherwise (in)active (when was he/she
> last active either or the list or doing a commit, ...)
> - file a port abandoned ticket in case it seems appropriate
> 
> And maybe we should, just once:
> - collect a list of all maintainers
> - remove those whose last commit was within the last year
> - send an email to everyone else, asking if they are still maintaining
> their ports, repeat that email two or three times
> - remove everyone else whose email bounced or got no reply
> 
> But let's not ignore our existing maintainer policy just because we
> have issues elsewhere.
> 
> Our main problem are tickets which have been opened for 2-10 years. I
> really really really do appreciate the work you Perry are doing to our
> PR queue, but waiting 72 hours won't actually make the PR queue any
> shorter. Yes, there might be 30 PR open for the last three days, but
> those are usually not the problematic ones. After three days it's
> perfectly fine to merge them.
> 
>> The fraction of the time when someone
>> ever comments on a proposed update is very, very low.
>> I don't know why this is, but it's indeed the case. Keeping MacPorts functioning
>> requires that we operate even in the presence of this. openmaintainer means that
>> we can keep the pull queue reasonably clear when we have relatively high
>> assurance that an update is harmless and well tested. If it becomes meaningless,
>> we'll have a lot of trouble functioning. If people don't want others touching their
>> ports, they should remove openmaintainer and not insist that it means the same
>> thing as closed maintainer.
> 
> Maybe we can discuss this again later, but let's first fix the real
> problem (clean up the inactive maintainers etc) ...
> 
> Mojca
> 
> PS: I would actually be in favour of replacing openmaintainer by
> closemaintainer. That is: to any port with missing "openmaintainer"
> keyword, add "closemaintainer". Then remove the "openmaintainer" tag
> from all other ports. The we wouldn't need to ask each individual
> first-time contributor to please add the "openmaintainer" tag.
>