I started contributing to open source about a year ago, and I've started a couple of projects myself so it's kind of weird I'm having this thought now.
I recently started a project and I expect to receive PRs to the dev branch. But what are the best practices to ensure that I'm not merging code that doesn't do what it's supposed to do (I don't mean broken code this time cos most can be handled with testing) into the same stream everyone is working on which is dev.
Which leads to my question or thought, whats actually the best approach to accepting PRs to a project or how do you currently do it? Because dev automatically becomes where everyone's changes end up before any release or something like that.
Top comments (5)
Two essential things:
Beyond that, you can automate test coverage reporting with something like Coveralls. If a pull request drops coverage substantially, that's a sign that it contains functionality which isn't being tested and should be considered unpredictable.
YESSS to what Dian said before. 😊
Besides the technical aspects: as a maintainer of an open source project you might end up coding less but instead responding to issues, pull requests (...) more - depending on your projects popularity.
So it'd be beneficial to reduce the volume of unwanted contributions in the first place. While this may feel unkind at the beginning, it's actually helpful for both parties. To do so you should clearly communicate your expectations and explain your projects's process for submitting and accepting contributions in your contributing guidelines. Writing things down is one of the most important things you can do as a maintainer.
Also, maybe these open source guides are interesting to you?! They provide some further useful and valuable tips for open source project maintainers, and contributors. 🙂
Contributing guidelines are a huge asset! They help people interested in submitting pull requests figure out what they need to do without interrupting you, and if someone submits a PR that doesn't fulfill your requirements (code standards, tests, documentation) you can just point them there. GitHub recommends a CONTRIBUTING.md in your repo root which seems like a sensible standard.
Consider Scope
Ask yourself honestly.
"Is it an acceptable addition, does it fit within the project's intended functionality?"
It's OK to reject a PR if it goes in an unwanted direction. OSS projects can suffer from feature creep too if you're not intentional about acxepting contributions.
Review the code
Like review every detail until you understand it fully. Itntakes practice to get good/fast at this but it's a necessary skill for a maintainer.
Document/automate away points of friction
PRs introducing regressions, add tests and require PRs to include tests.
PRs keep introducing stylistic changes that add no value, add linting as a required step. This can waste a LOT of time, especially with inexperienced contributors.
You say dev is done on 'dev', so you follow a specific workflow (ex GitFlow). Document that in CONTRIBUTING.md. While you're at it, document your entire development process in a step-by-step manner that users will understand.
Automate Automate Automate
CI is also a good way to automate all these checks. So every PR pushed to runs through a full suite of checks (ie test, lint, etc).
The more of this you take care of early on, the less effort you'll have to waste later dealing with poor quality contributions.
If time permits, you should tinker around with a PR locally before you make a decision. You can always ask the contributor is clarify what their PR will do and let them know that while their PR is accepted, you will make additional changes to it to fit your need.