Nueva Handbook

# Code Reviews

# How we should do Code Reviews

# Proposal when and how

Every project has a developer and a reviewer assigned. If there are multiple developers then they review each others code. When the developer think it is necessary that the reviewer has a look on the code the developer creates an issue on the project board and assigns it to the reviewer. If necessary the developer adds a short description what the feature is all about he also should make clear what should be reviewed maybe something is not 100% finished but a review already makes sense. It is Important that the review is not to big otherwise it is hard to review it. Every developer and reviewer is self organized and decides for themself when its best for the review and how long it takes. An approach could be have a blocker in calendar which is about one and half hour per week.

Here is a proposal how a review could go:

  1. The developer finish task and create a merge request.
  2. The merge request is deployed somewhere tbd!!.
  3. The developer assigns an issue on the code review board for the reviewer.
  4. When the code reviewer has time he reviews the code. And write suggestion for improvement as comments. (Because the branch is deployed somewhere he can also check how the code behaves)
  5. The code reviewer and the developer discuss shortly the suggestion.
  6. The Developer implements what is necessary.
  7. If the developer want another review start over at point 3

# When to create a mr

  • new complex vue component which is not in the start or library
  • when you are unsure about your implementation
  • when you have a complex component from pattern library or starter and made quite a change.

# On What should we focus in Code Reviews

  • How the code logic works in whole app.
  • If the requirements are met.
  • tipps für einfacher machen
  • fehler im code
  • struktur vom code
  • ist der code verständlich variablen naming

# What we not should do in code review

  • Catching bugs. I think automated tests and using the app are much better ways to find out how code actually runs.
  • Enforcing basic style rules. I think linters are much better for this.

# What should a developer do before assigning a code review

  • The whole feature should be implemented and all requirements are met.
  • The Developer should also do a self review to avoid having unused code or typos
  • Also do a short testing on the pre deployed version.

# Helpful questions for the code reviewer

  • Does this code accomplish the author’s purpose? Every change should have a specific reason (new feature, refactor, bugfix, etc).
  • Ask questions. Functions and classes should exist for a reason. When the reason is not clear to the reviewer ask.
  • Think about how you would have solved the problem. If it’s different, why is that? Does your code handle more (edge) cases? Is it shorter/easier/cleaner/faster/safer yet functionally equivalent? Is there some underlying pattern you spotted that isn’t captured by the current code?
  • Do you see potential for useful abstractions?
  • Think about libraries or existing product code.
  • Does the change follow standard patterns?
  • Think about your reading experience. Did you grasp the concepts in a reasonable amount of time?
  • Does this code have TODOs? TODOs just pile up in code, and become stale over time. Have the author submit a ticket on GitHub Issues or JIRA and attach the issue number to the TODO.

# Todo

Rahmenbedingungen für mr stellen wie grösse zeit und so weiter

# Sources