Vector is a tiny codebase currently (as compared to other repositories Web stewards, especially MobileFrontend and MinervaNeue). As such, it's ours to lose. If we merge YOLO "hero" commits that scramble the codebase, accept "rockstar" hackathon patches that add months of technical debt, or otherwise +2 patches that violate presently implicit standards, we'll never be able to recover and will succeed only in recreating our mistakes from previous projects. This should be well within our realm of influence even with tight schedules. Where automated tests cannot be employed, agreed and formally documented contribution standards in the repo will help us recognize the shortcomings in these patches and point others to them for easy identification.
This task encompasses the work to set some basic, uncontroversial coding standards in writing as part of the Vector readme file. For example, coding guidelines around adding unit tests, keeping documentation current, etc. MobileFrontend presently requires a threshold for unit tests but, regardless, agreeing on the principle in the conventions is important too. Broadly, I hope that we can agree that patches will be iterated on until they're of great quality so that incomplete patches don't slip through to master and mistakes from other projects are repeated. In a nutshell, don’t write bad code and certainly don’t merge it.
It should be fast to iterate on conventions and prove their usefulness within a single project, Vector. Most of these lessons, once polished and proven, can then be migrated to broader coding standards across repositories at the Foundation.
For automating some norms, @phuedx mentioned Danger JS as a possibility.
AC
- Team is solicited for their top code quality requirements on Slack and super-happy-dev-time
- Readme is updated. This doesn't need to be exhaustive and concise writing is preferred
- Mustache variable naming convention is clarified, example: https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/Vector/+/554284/3/includes/templates/VectorTabs.mustache@1
QA
No functional changes so no QA needed.