I'm not going to lie, opinions are like Arseholes, everyone has one. It would have been acceptable but the objectives of code reviews are simple. In general, not only is to get the best possible pull request, but also improving skills of the fellow contributors. Easy enuf, except we need a set of ground rules.
Etiquette
The first thing we're going to need is a contributor etiquette. I like the idea of Jante Law. It gives everyone a very Egalitarian feel since you are not to think you're anyone special or you're better than everyone else, as they teach in Scandinavian schools.
Workflow
The next is the release engineering strategy. All we need is a version control tool and git is the mainstream thingy for that. But some of the branching strategies are pretty horrendous so you have to keep the branch integrations to minimum.
Trunk-based development seems extraordinarily uncomplicated but there are more to consider.
- Branch policies for protecting the default branch.
- Naming conventions for branches.
- Pipelines for release engineering.
Changesets
Some organizations often call this a Pull request or a Changelist. And it has to be small-enuf to one self-contained change. Unfortunately, no playbook has a recipe for this so I'm going to have to let individual contributor decide the size of the pull request.
Once the pull request is ready to review, they have to describe it, specifically what has been done and why it was made as-is. This can be tricky, so a conventional-based description seems like a great way of adding structure and meaning to it.
Design and Health
This covers a lot of ground and can be challenging enuf. Unfortunately, you can only boil so many oceans at the same time, so what does each reviewer should look for in a pull request? In general, readability and maintainability would hint a few mistakes here and there, but there are more to review.
- Intended functionality and the design.
- Over-engineering and complexity.
- Clear naming and conformance to style guide.
- Parallel programming and thread safety.
- Sufficient exception handling and logging et al.
Style Guide
Yes, a homegrown standards and conventions isn't such a great idea. The major issue is that it would demand an intense concentration to keep up with each tech stack without giving away the egalitarian culture completely since you would want to Dictate-It-All. And you do want contributors to maintain consistancy of the code. This may be difficult. I need a community-adept style guide to start with.
Faster Reviews
A pull request won't get reviewed itself. And a contributor must intervene at some point. I can review shortly after it comes in. But that seems a little unproductive since it takes time to get back to The Zone after been interrupted. So how fast should code reviews be? One business day or 24 hours sounds reasonable but there is more to this.
- Professional courtesy when pointing out problems.
- Code analytics for obvious smelly codes, anti-patterns, conformance to standards, et al.
Summary
All we need is non-opinionated guidelines for all kinds of contributors, so in doing a code review, you should value for:
- Technical facts and data over opinions and personal preferences.
- Style guides over personal preferences.
- Software design principles over opinionated-design.
- Consistency of the code over personal preferences.
- Team velocity over personal velocity.
/KP
PS: You can find more about the topic in my curated list of standards, practices, guides, and discussions on my awesome list #33.
Top comments (0)