Cogito, Ergo Sumana for 2017 April 4 (entry 0)

# (1) 04 Apr 2017, 12:37PM: How to Teach And Include Volunteers who Write Poor Patches: You help run an open source software community, and you've successfully signalled that you're open to new contributors, including people who aren't professional software engineers. And you've already got an easy developer setup process and great test coverage so it's easy for new people to get up and running fast. Great!

Some of the volunteers who join you are less-skilled programmers, and they're submitting pull requests/patches that need a lot of review and reworking before you can merge them.

How do you improve these volunteers' work, help them do productive things for the project, and encourage and include them?

My suggestions for you fall into three categories: helping them improve their code, dealing with the poor-quality pull requests themselves, and redirecting their energies to improve the project in other ways.

Teaching them to improve their code

• Collect and suggest relevant learning resources, like certain talk recordings or freely available articles/exercises (e.g. The Architecture of Open Source Applications), and ask them to come back after they've watched/read/done them. Example: Zulip's collection.
• If developers have trouble writing good comments and commit messages, or diving into the codebase to find relevant files and commits, point them to my blog post "On the scientific method and usable history". It explains why it's important to do that, and gives them pointers.
• Ask more experienced contributors to pair program with them, both as leader and as follower. Here are a few tools to help.
• Run live coding exercises, over chat or video, where an experienced developer speaks aloud as she writes a bugfix, including all the little steps like searching for related commits, setting up and running tests, etc. This enables newer developers to learn a lot of tips that help them work faster and write higher-quality code. I've done this at Wikimedia with live video and we use Zulip for a live text approach (see Alicja Raszkowska's transcript and notes of one such session).
• If a big problem with their submissions is poor English writing skills, run some English tutoring sessions.

Dealing with poor patches themselves

• Write a code review guide (example from Zulip) and point to it liberally when giving criticism to contributors.
• While reviewing pull requests, use GitHub saved replies as much as you can. Point to coding standards, guides to rebasing, etc. Here's one set you can copy and modify.
• Follow Sage Sharp's advice on three-phase review, and consider using nicely phrased labels/tags in your patch tracker to indicate what phase the PR is in. Then you can, while reviewing, search for particular labels and concentrate on a class of patches at once.
• Be kind even as you criticize (see this Zulip example) and prioritize getting back to people fast, to signal that you genuinely want them to come back with a better patch (see this Mozilla analysis).

Using their knowledge and curiosity to improve the project in other ways

• Ask these developers to write "discovery reports". They're already user-testing your developer onboarding process; ask them for their experiences, so you can find and fix pain points.
• Ask them to run through some manual testing (example manual testing guide from Zulip), and to tell you how long certain kinds of tests took, so you can get bug reports and improve the docs.
• Ask them to teach about your project in their communities -- to develop learning and presentation materials and speak at meetups. You may have just found your most enthusiastic marketer.

This list is absolutely not the be-all and end-all for this topic; I'd like to know what approaches others use.

Thanks to Noah Swartz for starting a conversation at Maintainerati that spurred me to write this post.

---

Comments:

Posted by AJ Jordan at 05 Apr 2017, 02:38PM

I've also found that it's nice to integrate code correctness and style checks into your test suite. In the pump.io project, if your code doesn't meet our style guide, the Travis build will fail. This reduces the contributor's dependence on the maintainer, which is good on both sides - the maintainer can not even bother to examine style and just focus on the code, and the contributor doesn't have to wait for the maintainer to tell them about cosmetic problems.

This also has a nice side benefit, especially in understaffed projects where there aren't enough people to do code review on all patches, in that it pushes the _maintainer_ to also write better code. I've caught loads of linter and style checker violations that I checked in without noticing, because it broke the build.

---