Hacker News Reader

Comment by timthelion | Hacker News Reader

My feeling is that this is actually a problem with github and the "pull request" method specifically. All of the back and forth, waiting for the contributor to update commits is a huge time sink for me. It takes 10x as much effort to merge a small pull request than to actually write the code myself.

The solution I see, is to make it so that rather than commenting on pull requests, I "adopt" the pull requests. I merge them into a private branch, fix the spelling errors and the "bad" variable names, and then I push the bundle. For some reason, most contributors want to have a clean commit history and don't like this method. I agree that it is messy. I think that my "solution" points to the need for a better solution. Either one implemented by github or one implemented by a competitor which destroys github.

View on HN

I often do this. I will merge the PR with my fixes, and leave a comment for the new contributor letting them know the fixes I made (so they can do it themselves next time before making the PR).

I do this too. But it just doesn't feel natural on github. I feel like I'm doing something that is "unusual".

I'd suggest adopting a model that works for your workflow and your project goals, and asking contributors to adopt it as well. Github can also help accommodate this if you want. Example:

- Your project has two mainline branches, one against which development is done and the other from which releases are cut.

- The development mainline is the default branch

- this is the first thing contributors see, and the first branch PRs target

- You "adopt" PRs into the development branch, and when you feel the work is suitable, release it to the release mainline branch

Well, the method still isn't perfect, due to the lack of easy, well formated, ability to communicate to the contributor why the PR's commits were changed. The current UI creates a default culture of really tense back and forth nitpicking comments.

I don't agree. If the PR you're reviewing is expected to be merged into a development branch, you can either:

1. Merge without comment. Unless the contributor is closely following development, they won't even notice what you do with the contribution between your development and release branches. 2. Merge with a comment that you'll be cleaning it up before release, and if you want the person to continue contributing explain what you'll be doing.

Differentiating stewardship from producing solutions can go a long way toward eliminating debate over minutiae.

If there's still tense back and forth, it's probably over approach, understanding of the problem, or awareness of consequences. In none of those cases should the PR be merged. If you want to engage the contribution, explain what the contributor missed and close the PR. If you don't think the contribution is going to be productive, just close it.

Depending on the size of the nits you might be able to merge as is, then open a new issue to address the nits.

And all of these work-arounds just prove that the system could use a little bit of work.

Isn't https://github.com/blog/2141-squash-your-commits suppose to fix the "merge commit makes ugly git history"?

Reply on news.ycombinator.com