Live data from Hacker News

Probability of acceptance of pull requests

paulmillr.com

51–57 of 57 posts

Re: Probability of acceptance of pull requests

#51
post #9

What is a CLA and why would a project want one / not want one? They seem like another hurdle for contributors?

http://lmgtfy.com/?q=code+CLA

That is such a lazy and non-helpful answer, especially considering that the poster didn't just ask what it was.

Re: Probability of acceptance of pull requests

#52
post #41

Earlier quoted context omitted.

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 wh…

One way I've worked around this is to merge the original pull request, then immediately force-push the fixed version. Still doesn't fix all of the UI metadata, but at least the pull request gets marked as merged.

Re: Probability of acceptance of pull requests

#53

Earlier quoted context omitted.

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 wh…

One way I've worked around this is to merge the original pull request, then immediately force-push the fixed version. Still doesn't fix all of the UI metadata, but at least the pull request gets marked as merged.

Only works when you have push access to the repository that is the source of the PR. Normally maintainers don't have access to the repositories for all contributors.

Re: Probability of acceptance of pull requests

#54

Earlier quoted context omitted.

One way I've worked around this is to merge the original pull request, then immediately force-push the fixed version. Still doesn't fix all of the UI metadata, but at least the pull request gets marked as merged.

Only works when you have push access to the repository that is the source of the PR. Normally maintainers don't have access to the repositories for all contributors.

Er, no? You merge the unfixed version via the web UI (to get the PR marked as merged), then immediately force-push the fixed version to your own repo, not their's.

Re: Probability of acceptance of pull requests

#55

Earlier quoted context omitted.

Only works when you have push access to the repository that is the source of the PR. Normally maintainers don't have access to the repositories for all contributors.

Er, no? You merge the unfixed version via the web UI (to get the PR marked as merged), then immediately force-push the fixed version to your own repo, not their's.

Oh, I thought you meant updating the pull request by force-pushing on their branch. Your solution is racy, thus only acceptable for projects small enough that the likelihood of someone pulling during that window is sufficiently low.

Re: Probability of acceptance of pull requests

#56

Earlier quoted context omitted.

Er, no? You merge the unfixed version via the web UI (to get the PR marked as merged), then immediately force-push the fixed version to your own repo, not their's.

Oh, I thought you meant updating the pull request by force-pushing on their branch. Your solution is racy, thus only acceptable for projects small enough that the likelihood of someone pulling during that window is sufficiently low.

It's definitely not an option for very large projects, but projects small enough to have a very low chance of someone pulling in a several second window describes the vast majority of projects (and in general by the time it gets unsafe I'd expect fixing up incoming pull requests to be impractical time-wise anyway).

Re: Probability of acceptance of pull requests

#57

Earlier quoted context omitted.

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 repositor…

The problems with keeping the original commit and then putting the fixes on top are, (a) it makes the history harder to follow, (b) it makes it easy for someone to screw up a cherry-pick of a bug fix to a maintenance brance, since now you have to cherry-pick the bug fix plus the fix(es) to the bug fix, and (c) if the original patch had a potential bug that might cause the system to crash or otherwise malfunction, even if you fix it in the subsequent commit, it makes "git bisects" more likely to yield false positives, or at least make things more confusing and more difficult than it has to be.
Post reply on HN