Yesterday I re-read Best Practices for Maintainers (of open source) which I’d highly recommend reading if anyone hasn’t read it yet. Overall I think we do a pretty good job, and certainly do better than many others in the open source space, but I think there are some areas we can improve on.
I’m going to highlight some quotes and discuss some context around points. Note that I’m not mentioning anything I think we’re already doing well, like having a strong automated test suite running with continuous integration:
Before reading: high level
Individual files in each repo
Since most of our open source eco-system is very modular, having things stored in individual files is a pain to maintain for us. The existing things we have are things like licenses and contributing guides which point to documentation, and I think that’s good.
I’m happy to write some sort of tool to automate releasing changes for all modules in the
silverstripe GitHub organisation, but only if we have a good reason to do it.
For example: Travis configurations aren’t consistent enough between repositories to automate at this stage (I don’t think).
My suggestions (skim read for TL;DRs)
I’ve marked my suggestions as sort of TL;DRs with the fire emoji.
Start by writing down the goals of your project. Add them to your README, or create a separate file called VISION. If there are other artifacts that could help, like a project roadmap, make those public as well.
Having a clear, documented vision keeps you focused and helps you avoid “scope creep” from others’ contributions.
Our documentation is good - we’ve even had people say they think it’s awesome.
I wonder whether we should add a TL;DR at the top of the contributing code guide in the docs. This could even have a summary of the product’s “vision” at the top.
Suggestion: Add TL;DR product vision to the top of the contributing guide (3 or 4 bullet points)
Here are a few rules that are worth writing down:
- How a contribution is reviewed and accepted ( Do they need tests? An issue template? )
- The types of contributions you’ll accept ( Do you only want help with a certain part of your code? )
- When it’s appropriate to follow up ( *for example, “You can expect a response from a maintainer within 7 days. If you haven’t heard anything by then, feel free to ping the thread.”* )
I think the issue template provides value by explaining that we expect the affected versions and steps to reproduce to be provided, but I don’t think the PR template is very useful.
I see a lot of value in adding a pre-requisite checklist to pull request templates, for example:
- Pull request is linked to an issue describing what is being changed
- Where relevant, documentation has been updated
- New automated tests have been written where relevant, or tests are not required
- Commits are prefixed with the appropriate tag
If we add checkboxes we can see immediately from the repositories Pull Requests tab which issues are ready (from the initiator’s perspective at least).
There’s an amusing web wizard for generating these too.
Suggestion: Update the issue template to specify expectations around reviewing and timeframes (see next paragraph)
Suggestion: Update the pull request template to include a checklist of general requirements for every pull request
Suggestion: Synchronise these templates across all SilverStripe organisation repositories
3. Be proactive
Across the SilverStripe GitHub organisation we have 1,634 open issues at time of writing.
Here are some relevant quotes from the Best Practices for Maintainers:
Don’t leave an unwanted contribution open because you feel guilty or want to be nice. Over time, your unanswered issues and PRs will make working on your project feel that much more stressful and intimidating.
… ignoring contributions sends a negative signal to your community. Contributing to a project can be intimidating, especially if it’s someone’s first time. Even if you don’t accept their contribution, acknowledge the person behind it and thank them for their interest. It’s a big compliment!
If they don’t follow your rules, close the issue immediately and point to your documentation.
While this approach may feel unkind at first, being proactive is actually good for both parties. It reduces the chance that someone will put in many wasted hours of work into a pull request that you aren’t going to accept. And it makes your workload easier to manage.
Suggestion: close all open issues (?) and pull requests (!) that haven’t received any attention in the last n weeks/months:
- Clarify and document maintenance expectations around timeframes (discussed earlier)
- Give open PRs a cheeky bump now, then;
- Close everything
The last quote above the suggestion is the most prudent here - there are sooooo many stale issues and pull requests across our organisation. If we keep it all fresh and relevant, we’ll all feel much better about maintaining everything.
4. Issue triaging
I think out issue triaging system is awesome. Nice work @chillu!
This would be particularly important going into SilverStripe 5, since most of our repos don’t have labels for it yet.
Ingo mentioned a while back that
effort/easy might be better suited to be renamed back to
good first issue which is a GitHub wide label.
Suggestion: standardise labels across the organisation, post the configuration file for whatever utility we use on a public repo e.g.
5. SilverStripe 3 support
This isn’t from the maintainers guide, but think it’d be good to clarify expectations (again, discussed earlier) around SilverStripe 3.x
As far as I know, the official line is “we only accept security and critical bug fixes for SilverStripe 3.x now.” If this is true, let’s close everything that targets 3.x (note: previous suggestion in point 3 to “give open PRs a cheeky bump” should be considered here, as well as checking whether the issues affect v4).
At the end of the day, old stale pull requests are unlikely to be revived by their creators and are best cleared away.
- does this also apply to the supported modules - userforms, blog etc?
- how does this affect “commercially supported dependencies” like Symbiote’s multivaluefield which is only supported in SilverStripe 3.x?
- do these move to security support only?
- can we remove all minor version branches other than the last in the 3.x line from all modules?
- can we remove the SilverStripe 3.x compatible major version branch from all modules?
Suggestion: confirm SilverStripe 3.x expectations, close all 3.x issues and pull requests (that aren’t tagged as 4.x too), remove old branches, document these positions clearly in the contributing guide (see point 2 above).
Bit of a read, but I really think there are some valuable things we can achieve by actioning some of these points.
Would be great to get everyone’s input on this at some point.