Code reviews and the pull request are the basic building blocks in many software engineers’ life ( ). They prevent bugs, mistakes, and help distribute the knowledge around a code base in small units of . They are, also — and specially code reviews — more an art than a science, and so everyone makes mistakes here. Things as random as the relationship that different contributors have between them, influence whether something is approved, overlooked, ignored or . The Workflow changes requests changes And so for all that it is very important that we try to have a policy than addresses all these different variables, and tries to optimize the user experience (with all that it entails) and developer productivity. I’ll cut to the chase now, if you want to know why it’s important, please keep reading. Detailed Analysis https://undraw.com The Policy Define before hand what is the threshold to approve or reject a PR and in which cases. For example: Will be rejected if the changes contain a look with a time complexity unnecessarily high; Approved if it’s a typo fix or a one-liner to enable a flag feature; Rejected if it’s a bug fix without a test, etc. 1. Have a policy. Use a linter. Anything. , , , …Preferrably something that automates these tasks and do them for you in your IDE or (bonus points) in a . Syntax errors may prevent the code from compiling in some languages or may be cause an exception when interpreted on your user’s session. Many of the times this can be prevented. 2. Stop reviewing for syntax errors. SwiftLint Eslint PyLint RuboCop pre-commit hook Code style might be a historic standard (Java), built into the language (Python) or non-existing (Javascript!). But in all of those cases it’s something that is very opinionated and doesn’t add value to the conversation. Adopt a tool (any tool) that lints or automatically formats code so that your team submits everything under the same format and you reduce all the noise. , , , Really just pick anything. 3. Reduce or eliminate the talk about style . Google-Java-Format SwiftFormat Prettier . Set up some scripts to run on continuous integration upon merge-request. These are much easier to setup nowadays, many times with options included in the control version provided –as in , recently supported by 🎉— You can check , for most teams. & are interesting products for mobile. If you are still looking for some free optionsyou should check or and automate some routines there. 4. Invest in a pipeline GitLab Scope.ink Circle CI Travis Bitrise BuddyBuild now.sh glitch.com Some changes are just not going to compile. If you have implemented some git-hooks and/or have CI you can rely on the language itself preventing from a whole range of mistakes. There is no longer the risk of passing the wrong parameters to a function, or the infamous can't read preperty foo of undefined. This can be extended to other principles: high coverage increases confidence, but does not guarantee quality. 5. Typed languages increase confidence. Few times they are something welcomed by the writer, they probably chose this approach because it was appealing to them in the first place. it must come with a compelling reason: performance, error prevention or maintainability. Maintainability is something that can become very opinionated very fast. in the source, and specially, the same PR. 6. No one likes comments about readability. If a comment is indeed needed A good rule of thumb is to not question readability if that code path is covered by a test . This is not to be overlooked. Some times other engineers may come and take a look at the code, leave comments to never return. This is a missed opportunity to add clarity for the next reviewers and the developer themselves. 7. Communicate clearly what’s the information you are missing to approve the changes a “small change”. In a big code source, this may imply having to make further small change in function calls, dependencies, unit tests. Even for tiny bugs and enhancements 8. Think twice about requesting Pull Requests & Reviews over time in Scope.ink Finally, if you are the contributor: Enforce some PR description or have a consistent way to track user stories with software changes. A basic description template can be just asking for a rationale and a test plan. 1. Have a hard reference in your PR of what you believe to get reviewers approval. They should be as small as the minimum functional change that makes sense. How do you know if you are writing too much, why bothering splitting PRs? Again, this is more an art than science, but you can use to study the amount of changes, files and folders in elaborated diagrams. 2. Make pull requests as small as possible. Scope Anatomy of a PR of Microsoft Terminal Now, how can I get more reviews? We can’t improve what we can’t measure. It’s important to keep track and visualize what’s going on to move forward in the right direction. Change our habits. And that’s what we do. You can choose however any other tool for this matter, but we bring the topic because it’s important to our vision. Scope 🔬 can do now so much more, but it was born as a tool to do a just a few things very well: and be able to benchmark our contribution to different projects. See how people make progress through time. How well does our onboarding work for software engineers - Keep ourselves accountable . In Scope the number of PRs gives you more points than their size! That helped us encourage people to split their PRs which makes them way more easy to approve - Compensate people for the good workEncourage smaller PRs . Sometimes it’s hard to stop what you are doing in your project to contribute to someone else’s. Many organizations (even the top in the industry) lack a way to recognize the work than our peers invest in reviewing. By visualizing the PRs at the same level as reviews, the team in started to give reviews twice as fast. - Encourage reviews Yeti Smart Home You can read more about Productivity . How To Measure Developer here · · 🐦 · Author of · 🏡 Jesús Darío Twitter 🔬 Scope Yeti Smart Home

Google

Microsoft

You Need to Make Better Code Reviews™

About Author

Comments

TOPICS

THIS ARTICLE WAS FEATURED IN

Related Stories

Measuring Developer Productivity

50 Shades of Lead Time: Measuring Each Part of the Development Process

A Code Review Checklist to Focus on the Important Parts

Best Practices, Guidelines and Tools for Code Reviews

Code Reviews: A Practice That Depends on Culture

Code Reviews: A Multi-Tool You Paid Too Much For and Now Hesitate to Throw Away

Measuring Developer Productivity

50 Shades of Lead Time: Measuring Each Part of the Development Process

A Code Review Checklist to Focus on the Important Parts

Best Practices, Guidelines and Tools for Code Reviews

Code Reviews: A Practice That Depends on Culture

Code Reviews: A Multi-Tool You Paid Too Much For and Now Hesitate to Throw Away

Light-Mode

Classic

Newspaper

Minty

Dark-Mode

Neon Noir

Minty

HN StartUps