Code reviews

When submitting your report, add a comment with the submission containing the URL to the review, such as: "Lars Larsson has reviewed the code of group X (url) and Kim Wang has reviewed the code of group Y (url)".

Each of you will have to do a code review of code written by another group. This is an individual choice, but you and your teammates should choose different groups. As you are allowed to re-use code written by other groups for Assignment 2, this might be a good opportunity to get familiar with code you might want to use.

Overall is supposed to take you about 30 minutes (not counting the time it takes you to choose and find the code). 

Do as follows

Find the fork you want to do a code review of.

Create a PR (pull request) from fork to Master, so everyone can write your fork review there.

C39AB8B7F48C48D2B3FB9F8A99A92223.png

D3D97D004A294A8A9B851C8ADC84F539.png


Add group number to PR title!

CA3D602A4E0A4525BE26CF1939E8E1C2.png

Find other groups' PRs through master repo

A90028BC05C24B70A7B34EDFB36A7444.png

Write your code review into the chosen group's PR.

  1. Read and understand the code on a high level.
  2. Write a short summary. (Descriptive)
  3. Write about limitations. (Descriptive)
  4. Write about strengths. (Descriptive)
  5. Possible suggested changes. Code quality, technical (Unity) or functional (Algorithm). (Constructive)
  6. Write line comments where you want to ask more detailed questions, point out things.

 

Should on average take around 30 minutes.

If you need a better sense of the overall idea behind code reviews check out this link.

https://google.github.io/eng-practices/review/reviewer/looking-for.html Links to an external site.

https://google.github.io/eng-practices/review/reviewer/ Links to an external site.

Examples

"This solution uses A* to find an initial path that is then used for guiding the sampling for RRT.

Efficiency might be a concern with a solution like this because ... .

The method produces a drivable path due to the RRT component ... .

Might want to consider path smoothing as RRT can still produces drivable but curved paths. " (Overall)

 

"It might be easier to do X in Unity by using Y" (Technical)

"You can reduce complexity by N if you do Z" (Algorithm)

 

Inspiration for what and how to comment:

https://github.com/reviewpad/code-review-tips/ Links to an external site.