One morning I found a little time to work on nodemon and saw a new pull request that fixed a small bug. The only problem with the pull request was that it didn’t have tests and didn’t follow the contributing guidelines, which results in the automated deploy not running.
The contributor was obviously extremely new to Git and GitHub and just the small change was well out of their comfort zone, so when I asked for the changes to adhere to the way the project works, it all kind of fell apart.
How do I change this? How do I make it easier and more welcoming for outside developers to contribute? How do I make sure contributors don’t feel like they’re being asked to do more than necessary?
This last point is important.
The real cost of a one-line change
Many times in my own code, I’ve made a single-line change that could be a matter of a few characters, and this alone fixes an issue. Except that’s never enough. (In fact, there’s usually a correlation between the maturity and/or age of the project and the amount of additional work to complete the change due to the growing complexity of systems over time.)
A recent issue in my Snyk work was fixed with this single line change:
In this particular example, I had solved the problem in my head very quickly and realized that this was the fix. Except that I had to then write the test to support the change, not only to prove that it works but to prevent regression in the future.
My projects (and Snyk’s) all use semantic release to automate releases by commit message. In this particular case, I had to bump the dependencies in the Snyk command line and then commit that with the right message format to ensure a release would inherit the fix.
All in all, the one-line fix turned into this: one line, one new test, tested across four versions of node, bump dependencies in a secondary project, ensure commit messages were right, and then wait for the secondary project’s tests to all pass before it was automatically published.
Put simply: it’s never just a one-line fix.
Helping those first pull requests
Doing a pull request (PR) into another project can be pretty daunting. I’ve got a fair amount of experience and even I’ve started and aborted pull requests because I found the chain of events leading up to a complete PR too complex.
So how can I change my projects and GitHub repositories to be more welcoming to new contributors and, most important, how can I make that first PR easy and safe?
Issue and pull request templates
GitHub recently announced support for issue and PR templates. These are a great start because now I can specifically ask for items to be checked off, or information to be filled out to help diagnose issues.
Here’s what the PR template looks like for Snyk’s command line interface (CLI) :
- [ ] Ready for review - [ ] Follows CONTRIBUTING rules - [ ] Reviewed by @remy (Snyk internal team) #### What does this PR do? #### Where should the reviewer start? #### How should this be manually tested? #### Any background context you want to provide? #### What are the relevant tickets? #### Screenshots #### Additional questions
This is partly based on QuickLeft’s PR template. These items are not hard prerequisites on the actual PR, but it does help in getting full information. I’m slowly adding these to all my repos.
In addition, having a CONTRIBUTING.md file in the root of the repo (or in .github) means new issues and PRs include the notice in the header:
For context: semantic release will read the commits in a push to master, and if there’s a
feat: commit, it’ll do a minor version bump. If there’s a
fix: it’ll do a patch version bump. If the text
BREAKING CHANGE: appears in the body of a commit, it’ll do a major version bump.
I’ve been using semantic release in all of my projects. As long as the commit message format is right, there’s no work involved in creating a release, and no work in deciding what the version is going to be.
Something that none of my repos historically had was the ability to validate contributed commits for formatting. In reality, semantic release doesn’t mind if you don’t follow the commit format; they’re simply ignored and don’t drive releases (to npm).
I’ve since come across ghooks, which will run commands on Git hooks, in particular using a
commit-msg hook validate-commit-msg. The installation is relatively straightforward, and the feedback to the user is really good because if the commit needs tweaking to follow the commit format, I can include examples and links.
Here’s what it looks like on the command line:
...and in the GitHub desktop app (for comparison):
This is work that I can load on myself to make contributing easier, which in turn makes my job easier when it comes to managing and merging contributions into the project. In addition, for my projects, I’m also adding a
pre-push hook that runs all the tests before the push to GitHub is allowed. That way if new code has broken the tests, the author is aware.
To see the changes required to get the output above, see this commit in my current tinker project.
There are two further areas worth investigating. The first is the commitizenproject. Second, what I’d really like to see is a GitHub bot that could automatically comment on pull requests to say whether the commits are okay (and if not, direct the contributor on how to fix that problem) and also to show how the PR would affect the release (i.e., whether it would trigger a release, either as a bug patch or a minor version change).
Including example tests
I think this might be the crux of problem: the lack of example tests in any project. A test can be a minefield of challenges, such as these:
- knowing the test framework
- knowing the application code
- knowing about testing methodology (unit tests, integration, something else)
- replicating the test environment
Another project of mine, inliner, has a disproportionately high rate of PRs that include tests. I put that down to the ease with which users can add tests.
The contributing guide makes it clear that contributing doesn’t even require that you write test code. Authors just create a source HTML file and the expected output, and the test automatically includes the file and checks that the output is as expected.
Adding specific examples of how to write tests will, I believe, lower the barrier of entry. I might link to some sort of sample test in the contributing doc, or create some kind of harness (like inliner does) to make it easy to add input and expected output.
Fixing common mistakes
Something I’ve also come to accept is that developers don’t read contributing docs. It’s okay, we’re all busy, we don’t always have time to pore over documentation. Heck, contributing to open source isn’t easy.
I’m going to start including a short document on how to fix common problems in pull requests. Often it’s amending a commit message or rebasing the commits. This is easy for me to document, and will allow me to point new users to a walkthrough of how to fix their commits.
In truth, most of these items are straightforward and not much work to implement. Sure, I wouldn’t drop everything I’m doing and add them to all my projects at once, but certainly I’d include them in each active project as I work on it.
- Add issue and pull request templates.
- Add ghooks and validate-commit-msg with standard language (most if not all of my projects are node-based).
- Either make adding a test super easy, or at least include sample tests (for unit testing and potentially for integration testing).
- Add a contributing document that includes notes about commit format, tests, and anything that can make the contributing process smoother.
Finally, I (and we) always need to keep in mind that when someone has taken time out of their day to contribute code to our projects—whatever the state of the pull request—it’s a big deal.
It takes commitment to contribute. Let’s show some love for that.