Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Probability of acceptance of pull requests

paulmillr.com

Post reply on HN
Print view View original

41–50 of 57 posts

Re: Probability of acceptance of pull requests

#41

Post by caf »

jedbrown wrote: post #35
caf wrote: post #22

What would increase the accept rate greatly would be if there was a function to accept-pull-into-new-branch, which creates a new branch and applies. The maintainer can then apply the necessary changes/corrections/documentation updates, get the branch tested, then merge it back if all goes OK.

$ git fetch url-or-remote their-branch-name:my/better-branch-name $ git checkout my/better-branch-name $ hack, commit, etc

I'm aware you can do that with native git, of course - but that leaves the original pull request on github appearing "unapplied", doesn't it?

I meant that it would be useful to add a function for that workflow in the github UI.

Re: Probability of acceptance of pull requests

#42

Post by jedbrown »

caf wrote: post #41
jedbrown wrote: post #35

Earlier quoted context omitted.

$ git fetch url-or-remote their-branch-name:my/better-branch-name $ git checkout my/better-branch-name $ hack, commit, etc

I'm aware you can do that with native git, of course - but that leaves the original pull request on github appearing "unapplied", doesn't it? I meant that it would be useful to add a function for that workflow in the github UI.

If you edit commits in the branch, then yes, it appears as declined in the UI, which looks hostile when they are notified. I write a friendly message every time I do this, but it sucks to have to give that explanation and the UI metadata is still wrong. I asked for this on bitbucket, though it's currently on hold [1].

If you don't edit the existing commits, then it will show up in the github/bitbucket UI as merged when you eventually merge, regardless of what games you play with branches.

[1] https://bitbucket.org/site/master/issue/6704/supersede-pull-...

Re: Probability of acceptance of pull requests

#43

Post by jedbrown »

jjh42 wrote: post #30
tytso wrote: post #17

Earlier quoted context omitted.

It's so much better to send these sorts of proposal via e-mail. It's much easier to review the patches via e-mail, and everyone on the development community can make suggestions. Even so, it's not uncommon for patches to require three or four versions, and not just by first-time contributors. There are patches written by folks who have been working on ext4 for many years which still require three or four or more revi…

I think that depends a lot on the project. Projects that heavily use github tend to prefer a github pull request. It makes it easy to comment on the diff and integrates with Travis CI (integration tool) and other tools. It is easy to make multiple changes to the pull request before it is accepted so nothing about this system implies that pull request are always accepted without changes.

It's easy for the original author to make changes to the PR, but not easy for the maintainer or a third party to make changes. The micro-pedantry associated with spelling errors, tweaking commit messages, and trivial spacing/formatting issues can be more work for everyone when communicated through comments on PRs with the original author expected to apply and re-roll.

Re: Probability of acceptance of pull requests

#44

Post by Timothee »

What would be interesting to see would be merged vs. open vs. closed (as in rejected), and also the typical delay between the opening of the PR and the merging/closing.

Some projects are fast to close PRs if they don't fit their goals, some don't attend to the PRs at all and you have PRs lingering in the queue forever.

Two projects could have a similar acceptance rate with very different attitude: one could be on top of things but picky about what they merge in, while another is just not attending to the list except for the odd PR now and then. It seems that a project like the former would still be more interesting to participate in just because you could expect more feedback on what is wrong.

Regarding the presented data, I'm also surprised that it's that high for some project. With the way GitHub works, it's easy to create a pull-request without consulting anybody in the original project, so it's surprising that so many would match the projects' intentions.

Re: Probability of acceptance of pull requests

#45

Post by chrismorgan »

The conclusions don't seem reasonable to me. Higher acceptance rate is not necessarily better; it can also be that the quality of pull requests is too low: that the developers are instead discriminating in what they let into their project, and that the project benefits as a result of this.

Using it as a project quality metric is in this way similar to lines of code: higher doesn't necessarily mean better.

Re: Probability of acceptance of pull requests

#46

Post by icambron »

The question this raises for me is what the acceptance rate is as a function of the number of pull requests, or of the number of stars. A scatter chart would be cool here. I strongly suspect the acceptance rate goes down as the project gets bigger. That's some combination of the developers having less time to field all the requests and the more popular repos getting a lot of junk of PRs (anecdotal observation).

Re: Probability of acceptance of pull requests

#47

Post by Argorak »

kevingadd wrote: post #31

Vaguely related anecdote: I had a really bad experience trying to use node.js for a project. Eventually I figured out that the module I needed was just broken, so I decided to at least update the documentation. As it turned out, not only was it not documented how to update the documentation (...) but I had to go back and forth on undocumented style guide rules (for documentation that gets displayed in the browser any…

> I can only assume that the legal downside to not having a CLA signed for every single-line commit is enormous. Are there previous court cases in the US where not having a CLA got someone destroyed?

I don't know of any court cases, but I do know of several projects that were not able to change their license because they didn't have a CLA and couldn't contact all previous contributors. Thats the right that section 1 of the nodejs CLA gives them.

Fun fact: nodejs reserves the right to use your contribution under "(b) binary, proprietary, or commercial licenses".

Re: Probability of acceptance of pull requests

#48

Post by lucian1900 »

jedbrown wrote: post #43
jjh42 wrote: post #30

Earlier quoted context omitted.

I think that depends a lot on the project. Projects that heavily use github tend to prefer a github pull request. It makes it easy to comment on the diff and integrates with Travis CI (integration tool) and other tools. It is easy to make multiple changes to the pull request before it is accepted so nothing about this system implies that pull request are always accepted without changes.

It's easy for the original author to make changes to the PR, but not easy for the maintainer or a third party to make changes. The micro-pedantry associated with spelling errors, tweaking commit messages, and trivial spacing/formatting issues can be more work for everyone when communicated through comments on PRs with the original author expected to apply and re-roll.

Even when sending patches via email, I've always been asked by maintainers to do the trivialities myself and re-submit. I suspect at least part of the reason was education.

Re: Probability of acceptance of pull requests

#50

Post by bad_user »

jedbrown wrote: post #43
jjh42 wrote: post #30

Earlier quoted context omitted.

I think that depends a lot on the project. Projects that heavily use github tend to prefer a github pull request. It makes it easy to comment on the diff and integrates with Travis CI (integration tool) and other tools. It is easy to make multiple changes to the pull request before it is accepted so nothing about this system implies that pull request are always accepted without changes.

It's easy for the original author to make changes to the PR, but not easy for the maintainer or a third party to make changes. The micro-pedantry associated with spelling errors, tweaking commit messages, and trivial spacing/formatting issues can be more work for everyone when communicated through comments on PRs with the original author expected to apply and re-roll.

That's only partially true.

Pull requests on GitHub have to have an associated branch or fork in another repo. Even if you're not the original author, if you have access to it, you can do "git checkout" or "git clone", then you make the required changes and then on push the commits are automatically included in the pull request.

The only problem is that the maintainers do not have write access to the forked repositories of new or casual contributors. So if you're in a rush to change something, you have to fork that branch, make changes and then merge it, while ignoring the original pull request (although if you place the issue number in the commit messages you push, they'll get referenced on the associated page).

However, the commits logs made by the wannabe contributor are still in the history, so proper credit/blame is given where it is due, unless you rebase of course. And this isn't something you can say about diffs sent by email.

Also, GitHub sends you emails on pull requests. And you can reply by email too.

What I like about GitHub Pull requests is that you have a link you can give to people for review or that you can post wherever, a link that gives you a descriptive diff of what happened. Plus, the discussions around that pull request are left there, included in that link, left for everybody to see, instead of turning into a private conversation that nobody else will know about (incidentally, it's also the reason why I think native apps will never beat web apps, as long as native apps don't have URLs for referring to content or state within the app). This to me is useful enough to prefer it over diffs by email.

EDIT: there is one thing about GitHub I don't like - you cannot give write-access to people, while preventing them from pushing to master. If this where possible, you could give everybody access to push, except for master, in which case the above mentioned issue with pull requests would be a non-issue!

Post reply on HN