Closing pull requests

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Closing pull requests

Mojca Miklavec-2
Dear Ken,

I've seen that you closed a number of your own pull requests for
probably no good reason other than being disappointed that your patch
wasn't committed faster.

We are heavily understaffed. Not so much understaffed by the number of
committers (there are plenty in principle), but mostly by the
cumulative amount of time people spend reviewing others' work. Many
committers update or fix their own ports when they come around it, but
reviewing other people's work is something where we desperately lack
the workforce.

We have roughly 5000 open tickets on Trac. Most of them don't have any
patches, but just to give you the idea.

An additional problem is that we now have two types of otherwise top
contributors who are held back by "alien technologies". Even for me as
a great fan of GitHub, merging pull requests in a "proper way"
(without merge commits, getting the right committer's email address,
sometimes minor changes are needed) is quite a bit of a pain.

To me a closed pull request is either:
- an "user error" when the commit is in fact merged, but GitHub
doesn't properly recognise it as such
- a clear sign of rejecting the PR, meaning that patch was not
appropriate for whatever reason

Yours don't fall into either of those two categories.

Closing it just because one feels bad for not getting it accepted
earlier is super bad in my opinion. My personal policy is not to do
anything about the PR if it's closed by the contributor, so I won't
reopen it or work on it even if I feel that it should be. But mostly,
people don't have time to dig through old tickets just for the sake of
researching if they happen to have time to resurrect a closed ticket
(there are enough open ones to wok on).

In one case you mentioned that Ryan would have acted upon the ticket
if he wanted to. Ryan is our top contributor (who feels much less at
home with git and in particular with GitHub in comparison to
subversion), who maintains over 1000 ports, but can no longer spend 16
hours per day working for MacPorts, so it's not fair "blaming" him for
not providing you the feedback fast enough.

Please reopen those tickets which you still consider worth merging.
You may send an email to the -dev list or ping developers on IRC to
remind them there's something you would like to see merged. But please
don't close tickets "just because".

Once you have sufficient track record of good contributions (I'm not
sure what the criteria is exactly), you may request commit rights and
then commit to the repository directly.

Mojca

PS: In one extreme case I waited *6 years* for upstream developers to
include my patch (which just added additional module to export a graph
in a different text-based format, without changing any core
functionality). Developers just thought that installing another piece
of software was too complex, so they could not test whether my export
created any useful result and claimed that anyone who wanted to use my
module could *easily* compile the software from source (which is super
easy, right?).
Reply | Threaded
Open this post in threaded view
|

Re: Closing pull requests

Ken Cunningham
I did close the PR for the ImageMagick openmp submit. I know Ryan hates PRs. I only generated the PR in the first place because I thought it would make life easier for him, which was my only goal.  I assumed these were one-click things on your end, as some of my PRs get committed in minutes. When I saw the PR was not helping, I left it here as a patch in the  original ticket, as he seems to prefer that, and all the info might as well be in one place for a searcher.

<https://trac.macports.org/ticket/54050#comment:6>

Similar with the fix for upc. I don't know where tenomoto is, but this one sat for about a month, and also has a ticket, so I left the fix there and closed off the PR as redundant. People will search trac when upc fails to build, and find the fix there.

darwinbuild-legacy:

Well, that one is a bit petty, I'll admit, and I closed that in the string of them. It's an ancient, rather messed up port that doesn't use the right compiler and would never in a million years pass a thorough inspection for a new submission today, but it works perfectly fine on the machines that it's targeted towards.. Jeremy asked me to see if it could be made available for another project I'm working on. I am curious why that "commit" button didn't get hit there..I demonstrated it built and worked fine on 10.4PPC and 10.5PPC -- .but from your note, I now realize committing a PR is quite a bit more involved than just popping a button, which is a pity. Now that I know that, I won't have expectations of anything.

One comment, though. I know about the manpower, and the volume. I get it. And I recognize all the work everyone does to keep things rolling (including what I do).

But if MacPorts is really trying to increase usage and get more participants, it might be difficult to sell that if submissions and fixes take months or even years to get in.

Just food for thought.

Ken
_______________________________________________
macports-dev mailing list
[hidden email]
https://lists.macosforge.org/mailman/listinfo/macports-dev