How to write the perfect pull request (2015)
github.blog
How to write the perfect pull request (2015)
1–10 of 40 posts
Re: How to write the perfect pull request (2015)
#2> Use emoji to clarify tone. Compare “:sparkles: :sparkles: Looks good :+1: :sparkles: :sparkles:” to “Looks good.”
Or, you know, write "Looks great!"
Exclamation mark is the OG emoji
Re: How to write the perfect pull request (2015)
#3This is a good article that covers how to communicate in a pull request. I think there are two other essentials in making good pull requests.
1. Good commit messages. Follow this guide: https://chris.beams.io/posts/git-commit/
2. Following the pull request workflow correctly. Follow this guide: https://github.com/susam/gitpr
Writing good and consistent commit messages make the commit log easy to read and search.
Pull request workflow is equally important. It is kind of a rules of engagement that co-developers follow to keep working on their stuff concurrently. It keeps unnecessary merges to minimum and keeps the commit history clean.
By following these two things, you are not only going to be nice to your co-developers but you are going to be nice to your managers and release departments too who could look at your commit log and figure out what bugs were fixed and what features were released.
Re: How to write the perfect pull request (2015)
#4Better support for pull request revision would be nice. In mailing lists, the entire patch set is resent when revisions are made. When I force push revised commits to my branch, the old commits are eventually garbage collected despite the existence of references to those commits in the pull request.
Also, it is important to discuss pull request etiquette. When is it appropriate to mention the maintainers? How to draw attention to a pull request?
Re: How to write the perfect pull request (2015)
#5Better support for pull request revision would be nice. In mailing lists, the entire patch set is resent when revisions are made. When I force push revised commits to my branch, the old commits are eventually garbage collected despite the existence of references to those commits in the pull request. Also, it is important to discuss pull request etiquette. When is it appropriate to mention the maintainers? How to draw…
It is a problem in mailing lists but GitHub has good tooling around it. Every time you force-push, GitHub keeps and provides direct links to the old commit, the new commit and the diff between the two commits. Every force push appears as its own diff on the GitHub PR page.
How is GitLab doing in this area? Does GitLab show diffs for force-pushes?
Re: How to write the perfect pull request (2015)
#6This is a good article that covers how to communicate in a pull request. I think there are two other essentials in making good pull requests. 1. Good commit messages. Follow this guide: https://chris.beams.io/posts/git-commit/ 2. Following the pull request workflow correctly. Follow this guide: https://github.com/susam/gitpr Writing good and consistent commit messages make the commit log easy to read and search. Pull…
I will elaborate why I think the two points in my comment above are important.
Git Rebase, Git bisect and other operations display the commit summary line when they get stuck with merge conflicts or find an issue, so I find good commit summary lines very helpful during those operations. Without good commit messages, resolving issues during those operations can get confusing. This is one of the reasons why writing good commit messages are important.
I see many developers working on their PR branch and constantly merging new commits from master into the PR branch as the master keeps moving ahead. It creates a big mess of merges in both directions-- (A) from master to PR branch during development and (B) later again from PR branch to master when the PR gets merged. The first set of merges from master to PR branch are totally unnecessary. It adds nothing meaningful to the commit history. They are just extra commits to scroll through while looking at git log. Every time the master branch moves ahead, just rebase your PR branch on master. The commit history remains clean and minimal. A good PR workflow teaches you that.
Having said that, merge commits from PR branch to master are totally fine. They do add something meaningful. They show the point at which a PR was merged into the main project.
Re: How to write the perfect pull request (2015)
#7Better support for pull request revision would be nice. In mailing lists, the entire patch set is resent when revisions are made. When I force push revised commits to my branch, the old commits are eventually garbage collected despite the existence of references to those commits in the pull request. Also, it is important to discuss pull request etiquette. When is it appropriate to mention the maintainers? How to draw…
It is a problem in mailing lists but GitHub has good tooling around it. Every time you force-push, GitHub keeps and provides direct links to the old commit, the new commit and the diff between the two commits. Every force push appears as its own diff on the GitHub PR page. How is GitLab doing in this area? Does GitLab show diffs for force-pushes?
> Every time you force-push, GitHub keeps and provides direct links to the old commit, the new commit and the diff between the two commits.
Not in my experience. Here's one of my pull requests:
https://github.com/mchehab/zbar/pull/64
The maintainer reviewed some of the changes and I revised my commits as a result. The review is correctly marked as outdated. Clicking on the file name tells me the commit cannot be found.
> We went looking everywhere, but couldn’t find those commits.
> Sometimes commits can disappear after a force-push. Head back to the latest changes here.
Re: How to write the perfect pull request (2015)
#8Earlier quoted context omitted.
It is a problem in mailing lists but GitHub has good tooling around it. Every time you force-push, GitHub keeps and provides direct links to the old commit, the new commit and the diff between the two commits. Every force push appears as its own diff on the GitHub PR page. How is GitLab doing in this area? Does GitLab show diffs for force-pushes?
> Every time you force-push, GitHub keeps and provides direct links to the old commit, the new commit and the diff between the two commits. Not in my experience. Here's one of my pull requests: https://github.com/mchehab/zbar/pull/64 The maintainer reviewed some of the changes and I revised my commits as a result. The review is correctly marked as outdated. Clicking on the file name tells me the commit cannot be foun…
Your pull request shows exactly what I am talking about. For every force-push it has direct links to the old commit, the new commit and the diff.
Pick the first force push in your PR. It says
> matheusmoreira force-pushed the matheusmoreira:binary-decoding branch from aec04b3 to 87a0b3c on 5 Nov 2019
Click on 'force-pushed'. That's the diff of force-push.
Click on 'aec04b3'. That's the commit before force-push.
Click on '87a0b3c'. That's the new commit in the force-push.
Re: How to write the perfect pull request (2015)
#9Earlier quoted context omitted.
> Every time you force-push, GitHub keeps and provides direct links to the old commit, the new commit and the diff between the two commits. Not in my experience. Here's one of my pull requests: https://github.com/mchehab/zbar/pull/64 The maintainer reviewed some of the changes and I revised my commits as a result. The review is correctly marked as outdated. Clicking on the file name tells me the commit cannot be foun…
Your pull request shows exactly what I am talking about. For every force-push it has direct links to the old commit, the new commit and the diff. Pick the first force push in your PR. It says > matheusmoreira force-pushed the matheusmoreira:binary-decoding branch from aec04b3 to 87a0b3c on 5 Nov 2019 Click on 'force-pushed'. That's the diff of force-push. Click on 'aec04b3'. That's the commit before force-push. Click…
You're right. I never realized those messages contained links. They do lead me to the old commits.
I don't understand why the code review can't find the commit though.
Re: How to write the perfect pull request (2015)
#10Discussed at the time: https://news.ycombinator.com/item?id=9717937