Photo by Clay Banks on Unsplash
Hi hacktoberfestees 👋
I am Jan and I maintain some repos on GitHub. They may not be popular and you may not have heard of them. However, I still have had some interesting experiences as maintainer with pull requests in various quality over the years.
Since it's hacktoberfest I'd love to give you some hints about creating a PR and increase the chances of it being merged.
My perspective as a contributor
First of all, I have been a contributor to various repos before I became a maintainer. I often felt overwhelmed by the whole complexity of what to consider in order to even just start working on a running dev-environment. On top of that there were so many rules and guidelines that I didn't cover them at all (simply overlooked them mostly).
I started my way through documentation and then picked-up minor issues, like fixing a typo or renaming a variable.
From there I stepped into further development, mostly on those technologies I actually work with every-day. By the time I also opened a few PRs that were not reviewable at all. They simply were not really tackling the issue or the code quality was just too bad.
However, I also learned from these unmerged or rejected PRs!
It showed me, what all these meta-processes of development were and how important they are when moving from "home-alone 1st person developer" to actually working together on code as a community.
This is also one of the reasons I wrote this guide: to make life easier for you; by pointing you at these things and showing you their importance. ❤️
My perspective as maintainer
At this point I'd like you to understand why it's important to see the maintainers perspective here:
- many of their repos are related to their work
- therefore the high priority tasks arise from that work
- resources for issues, that are not work related are limited
- this is a good chance for you to pick these issues and get involved
- however, reviewing and merging takes time for maintainers, too (read code, test locally sometimes, audit dependencies)
- maintainers often do this in their free time (yes, that time they should spend with their families, friends or just for themselves)
Therefore, consider this carefully:
If the maintainer thinks, your PR is way more work than benefit to them, then you might have little chances of this getting merged anytime soon.
This should not scare you off but instead be a guiding principle for your next PR. Fortunately, many good repos contain all necessary info to ensure this principle as I will show you in the next section.
What is the most important before start working
When picking a repo and issue then you may prepare yourself, before actually start to work on things. This saves you valuable time as you will also realize, whether the issue will fit your skill level and your own time constraints.
1. Read the issue carefully
Some issues are written with lot of information on what's the problem or how to reproduce a bug, if there is any. Others define specific feature requirements and boundaries.
Don't hesitate to ask if an issue is unclear or you're unsure about you correctly understood it's terms. The more info you can gather before working, the less likely you will work on things that aren't even required (which is a waste of your valuable time).
It is also an option to ask about the expected complexity. If the issue is created by the maintainer or project member then it's very likely they can estimate this as they know the issue's context very well.
3. Always talk about dependencies, before touching them
If you think the issue can only be resolved by introducing dependencies, update dependencies or remove/replace deprecated dependencies then first ask the maintainers about their opinion on this.
There are many issues involved with dependency management. Some can result in serious security risks, and it takes additional effort to review PRs with such changes. Opening a PR with dependency changes that have not been discussed before can be a no-go for some maintainers.
2. Read the contribution guidelines even more carefully
Especially bigger projects often have a very detailed contribution guideline in order to keep distributed development manageable.
Look out for (but not limited to):
- coding conventions (formatting, variable naming, flavors)
- how to install the dev and build environment in the correct way
- how to run tests and lint (for languages that require or support linting)
- how to document your work
- how to manage dependencies
- how to structure commits and to format commit messages
If you think there are topics uncovered then again, leave a comment in the issue and ask for clarification. Leaving a comment is much less work for the maintainers than reviewing a whole PR.
3. Keep up with the code of conduct
I should not need to mention it but: always be nice. Don't forget that written words don't contain the same nuances as spoken words and things can be misinterpreted easily. Many repos therefore use a "code of conduct" that approximates to make this issue manageable.
This also applies for the answers of the maintainers. Unless they are actually rude and offensive you should first ask for clarification before feeling offended and starting a flame war (which you shouldn't do anyways).
If it's clear that they are not friendly at all, keep your integrity, say thank you and leave them be (or take further measurements if they are openly offensive, discriminating, racist etc. by contacting GitHub).
When working on the code
Okay, you prepared well. Now it's time to keep focus and work on the code as professionally as possible.
1. Stay concise in what you are doing
Sometimes things get out of hand and you find three more new issues while working on one. Unless they don't stop you from continuing your work, take notes and discuss them with the maintainers in the PR description.
Never introduce things that were not covered by the issue description. If you fix a bug then don't introduce a new feature unless it's discussed first. If you introduce a new feature then don't introduce another feature just "because it was such a low-hanging fruit".
Proper review requires narrow scope and when you keep that scope then you have already increased your changes for a merge a lot.
2. Always make sure tests and linter pass
Avoid CI fails on opened PRs by always testing locally before committing. Often you can run the tests in parallel with a watch-mode that re-runs the tests in the very moment you saved your changes to the code.
If you can't get the tests running, re-read the development guide or contribution guidelines. If they won't help either - ask.
3. Don't change dependencies if it's not part of solving the issue
I already mentioned this but I just can't stop saying it again.
Sometimes there are PRs where the contributor thought, "hey this library just fits perfectly for this issue". However, under the hood the maintainers often knew this already but were also aware of the technical dept that is introduced by it.
Introducing technical dept may get your PR being left unmerged or being rejected with ask for major changes. It's therefore always important to discuss anything that involves working on dependencies!
4. Keep commit conventions or use understandable commits
Often the conventions are covered by the contribution guidelines. Otherwise, either ask for one or use some best practices. Definitely avoid commit message like
updated component xfixed yadded zremoved a
They are too generic to understand what happened when searching through the git history. Think this way:
How to describe this commit in a way, that I can find it fast and with low effort when searching in 1+ year for a certain change.
When opening the PR
You finally finished your work or a great portion of it and want to make the final step to open the PR.
1. The CI is always right (debatable, but that's the way it is)
Make sure the CI passes, otherwise many maintainers will not even look at the PR. If you can't see the reason why the CI fails (for example because you can't look at the log) you may ask the maintainer about it's reasons. However, again make sure that tests, lint, build etc. work locally in a reproducible way. Often the error lies in there.
2. Expect the review to be rejected
This is not a bad thing! It's part of the quality assurance process. There are often minor issues, but sometimes the maintainers may not agree with your choice of a solution. This may occur, when your solution has negative implications for performance, scalability, security or introduces concepts that prevent long-term maintenance, like tight-coupling.
In such a case, don't panic or feel down but kindly ask the maintainers if they point you to a conceptual solution (like an article or some terms to search), then do your own research and update your code.
By doing so, you and the maintainers will create an unbeatable team, creating the best possible solution together and you will learn something as well!
3. Use draft PRs for larger or open-ended issues
This is a great way to show your "construction site" and give the maintainers opportunity to take a look at the current state without being overwhelmed by a huge PR with hundreds of lines changed.
Summary
Wow this is all so much, right? Let's wrap it up in a short checklist:
before coding
- read the issue carefully, ask for more info if anything is unclear
- read the contribution guidelines, development guide and code of conduct carefully, ask if anything is uncovered or not clear
- be nice and friendly at all times
during coding
- stay concise and keep focus on the issue
- make sure all tests run locally, before committing
- don't touch dependencies unless it's necessary part of the issue and discussed beforehand
- write understandable commit messages that help you in the future
when opening the PR
- Make sure CI passes
- expect multiple review cycles and rejections
- use drafts for larger work
Thank you and happy hacktoberfest coding! ❤️
I regularly publish articles here on dev.to about Meteor and JavaScript. If you like what you are reading and want to support me, you can send me a tip via PayPal.
You can also find (and contact) me on GitHub, Twitter and LinkedIn.
Keep up with the latest development on Meteor by visiting their blog and if you are the same into Meteor like I am and want to show it to the world, you should check out the Meteor merch store.
Top comments (0)