Making Better Pull Requests
Reading time: about 5 minutes
Published
Hactoberfest is here! Time to find those issues and participating repositories, write some code, and create pull requests! I've been eagerly waiting for October, mostly because of Hacktoberfest.
Last year, I participated in Hacktoberfest by being a contributor and a maintainer. If you're interested, you can read about my blog post about last year's experiences: Eevis Panula - Story of an accidental open source maintainer
This year, I decided to take it a bit easier. I mean, as mentioned in the blog post, last year we bought a house and were moving during October. Another challenging thing from last year was the maintainer part. I loved it, but it really, really stressed me out. So I'm not going to do it this year. And there's another piece to the puzzle this year as well: I'm switching jobs, so that's another reason to take it easy during this Hacktoberfest.
One big part of Hacktoberfest is creating pull requests. I remember the first time I made a PR in an open-source repository. I didn't know the repository's maintainers, and it didn't have any guidelines for pull requests, such as a pull request template. I had no idea what the maintainer expected from a PR. I felt super anxious.
If I had known back then what I know now, it would've been easier for me. So that's why in this blog post, I'm sharing some tips on how to make better PRs. These are the best practices I've picked up along the way. However, note that there might be some other conventions in the project you're contributing. Be sure to check them. There might be a pull request template or contribution guidelines.
The tips I'm going to give are the following:
- Keep the pull request in scope
- Give context to the reviewer
- Add a picture of the changes
- Pull request is a place for discussion
Keep the Pull Request in Scope
One reason for keeping the PR in scope is that smaller PRs are always easier to review than those with lots of changes. Sometimes, of course, the feature you are working on is a big one, and thus the PR gets enormous. However, keeping in the scope in those cases means that if you find things to fix or improve, I would suggest creating a separate PR if they're not part of the feature.
Sometimes it would be good to separate the feature into smaller PRs. The scope gets smaller, and thus there will be less code to review. In these cases, I would advise splitting the PR into logical parts. For example, if you are building an app for period tracking, and the feature you're implementing is a calendar view where the user adds their data, there are multiple ways to split that.
Maybe the first part would be creating the endpoint to the backend and then adding a simple way to input the data with just input fields (so, no calendar yet). Maybe you could combine these all into a feature branch in the end.
Whatever you decide to do, be sure to check what the conventions are in the project you're working on. If you're working with a team, you can reach out and talk with them. There might also be a document about contributing, where there might be something about this topic.
Give Context to the Reviewer
When I'm reviewing a pull request, I love seeing some context about the changes. It might be a link to an issue (with more info than a vague title), or it might be a text explaining the context in the PR description. Anyway, having that context helps me get into reviewing mode and know what I'm reviewing.
Continuing from the example from the previous section, I would briefly write about the calendar view as a goal or link to the issue and then explain what I did in the pull request. I would also add some notes about the decision to split the feature and maybe some words about what's not yet included.
Add a Picture of the Changes
When the pull request is about changing something in the UI, I love seeing a picture of the changes. It's part of the context, and I know what to look for when I pull the branch and test it.
Continuing with the example, after moving on to making changes to the UI, I would add screenshots of those changes to the pull request. I might consider adding a before picture when finally implementing the calendar view to see how the feature evolves. That, however, would depend on the situation.
Pull Request is a Place for Discussion
One piece of advice I want to give here is that pull requests are places for discussion. It's ok to submit a pull request with questions (you could use a draft pull request in Github for that). On the other hand, you'll likely get questions and comments. Maybe even request to change something. When you do, and if they're not clear, it's always ok to ask for clarification or the reasons for change requests.
Why am I pointing this out? Well, at the beginning of my career, I felt like all the things said were God's truth, and I, as a newbie developer, didn't have any place to ask or question anything. That was even if the reviewers were friendly and tried to facilitate an atmosphere where that would've been ok. I would've needed to hear this advice many more times back then for it to sink in.
Summing Up
In this blog post, I've discussed pull requests and how to make them better. The advice is subjective, and it's based on my own experiences, and you might have other opinions. That's ok.
I pointed out that it would be good to keep the pull request in scope and try not to do everything in one pull request. If you want to make some additional changes, I'd suggest opening a separate PR for that. Also, in the pull request description, give context about the pull request for the reviewer. Additionally, if there are changes in the UI, I suggest adding a screenshot of those changes. The last thing I pointed out is that pull requests are places for discussion.
Links
Cover photo by ThisisEngineering RAEng on Unsplash