V model (Forsberg & Mooz, 1991)

• each phase has output and a review process
  • errors are found at early stage
  • decreases the risk of failure
• testing is done in a hierarchical perspective

requirement analysis review

• can be discussed / reviewed
• even with a customer representative

architecture review

code review

def query_progress(user_id:int) -> float:
    # establish connection
    con= sqlite3.connect("data.db")
    # build query
    progress_query = f"""
    SELECT
        lesson / 50.0 AS progress
    FROM activity
    WHERE
        user_id = {user_id} AND
        result = 'success'
    ORDER BY
        lesson DESC
    LIMIT 1
    ;
    """
    # execute query
    res =con.execute(progress_query)
    progress=res.fetchone()[0]
    return progress

review types by formality

informal

• asking a colleague to have a look at the code
  • they express their opinion and that’s all
  • no documentation
  • no process
• pair programming is also a kind of constant informal review

walkthrough

• not a formal process / review
• led by the author(s)
• the author(s) guide the participants through the work product to achieve a common understanding and to gather feedback
• useful for higher level documents like requirement specification
  • e.g., risk storming, sprint review

technical

• less formal review
• led by the trained moderator or a technical expert
• often performed as a peer review without management participation
• issues are found by experts (e.g., architects, designers)
• technical reviews can vary from quite informal to very formal

(Fagan) inspection (Wikipedia contributors, 2021)

process phases

• planning: the inspection is planned by the moderator

• overview meeting: the author describes the background of the WP

• preparation: each inspector examines the work product to identify possible defects

• inspection meeting: reader reads through the work product, part by part and the inspectors point out the defects

• rework: the author makes changes to the work product according to the action plans from the inspection meeting

• follow-up: the changes are checked to make sure everything is correct

roles

• author: created the work product being inspected

• moderator: the leader of the inspection, who plans and coordinates it

• reader: reads through the documents, while the other inspectors then point out defects

• recorder: documents the defects that are found during the inspection

• inspector: examines the work product to identify possible defects

review types by formality – summary

code review – author’s perspective

you are not your code

code review – reviewer’s perspective

• pay attention to the way you are formulating your feedback
  • phrasing is crucial for your feedback to be accepted
• you and the author are in the same side
• the goal is to deliver higher quality code, not about arguing who was right

use I-messages

talk about the code, not the coder

ask questions

refer to the author’s behavior, not their traits

wrong
You are sloppy when it comes to following the style guide.

Can’t you just configure your IDE properly?

right
I believe that you should pay more attention to the style guide.

Try to enable the auto-formatting in your IDE.

OIR-rule of giving feedback

Observation
Describe your observations in an objective and neutral way. Refer to the behavior if you have to talk about the author. Using an I-message is often useful here.

e.g., “This method has 100 lines.”

Impact
Explain the impact that the observation has on you. Use I-messages.

e.g., “This makes it hard for me to grasp the essential logic of this method.”

Request
Use an I-message to express your wish or proposal.

e.g., “I suggest extracting the low-level-details into subroutines and give them expressive names.”

three filters for feedback

always ask yourself, if your feedback is true, necessary and kind

(from April Wensel (Wensel, 2018) via (Hauer, 2018))

• is it true?
  • avoid statements assuming an absolute truth
  • avoid the words “right”, “wrong”, “never”, “always” and “should”
  • refer to your opinion instead
• is it necessary?
  • does the demanded requested change make the reviewed code better
• it it kind?

praise

• don’t forget to express your appreciation if you have reviewed good code
• praising doesn’t hurt you but will motivate the author
• however, be specific and separate the prasie from the criticism

looks good to me

a review process

• is it understandable (clear)
• is it clean (no code smells)
• does it match the task?
• does it fulfill the task?
  • every DoD point covered?
• is it possible to improve?

how can you tell if the code matches the task?

how to write the commit message?

1. Separate subject from body with a blank line
2. Limit the subject line to 50 characters
3. Capitalize the subject line
4. Do not end the subject line with a period
5. Use the imperative mood in the subject line
6. Wrap the body at 72 characters
   • least important
7. Explain what and why not how
8. Reference the issue!
   • my addition for traceability
   • GH-33 (GitHub), GL-33 (GitLab),
   • or custom identifier for a project (Jira, JetBrains)

review in numbers

#include <stdio.h>
main()
{
    int a,b,c;
    int count = 1;
    for (b=c=10;a="- FIGURE?, UMKC,XYZHello Folks,\
    TFy!QJu ROo TNn(ROo)SLq SLq ULo+\
    UHs UJq TNn*RPn/QPbEWS_JSWQAIJO^\
    NBELPeHBFHT}TnALVlBLOFAkHFOuFETp\
    HCStHAUFAgcEAelclcn^r^r\\tZvYxXy\
    T|S~Pn SPm SOn TNn ULo0ULo#ULo-W\
    Hq!WFs XDt!" [b+++21]; )
    for(; a-- > 64 ; )
    putchar ( ++c=='Z' ? c = c/ 9:33^b&1);
    return 0;
}

#include "stdio.h"
int main (void) {
    int a=10, b=0, c=10;
    char* bits ="TFy!QJu ROo TNn(ROo)SLq SLq ULo+UHs UJq TNn*R\
    Pn/QPbEWS_JSWQAIJO^NBELPeHBFHT}TnALVlBLOFAkHFOuFETpHCStHAU\
    FAgcEAelclcn^r^r\\tZvYxXyT|S~Pn SPm SOn TNn ULo0ULo#ULo-WH\
    q!WFs XDt!";
    a = bits[b];
    while (a != 0) {
        a = bits[b];
        b++;
        while (a > 64) {
            a--;
            if (++c == 'Z') {
                c /= 9;
                putchar(c);
            } else {
                putchar(33 ^ (b & 0x01));
            }
        }
    }
    return 0;
}

references