Code Review Guide
Reviewing GitHub Pull Request (PR)
A Pull Request (PR) on GitHub is a request to merge changes from one branch into another. Here are the basic steps for reviewing a PR.
- Once the PR has been created, other teams or team members can review the PR on GitHub.
- Specific lines can be commented on and "Suggestions" can also be made for certain lines of code.
- Suggestions: This is a useful feature of GitHub. When a reviewer suggests a small change, they can do so directly in the PR comment. The creator of the PR can then accept this suggestion with one click and incorporate it into the PR.
- 🔗 GitHub: Commenting on a pull request (opens in a new tab)
💡
Sometimes it can be helpful to clone the code locally. If you choose this approach, make sure to install all necessary dependencies and take note of the code's documentation.
Things to Pay Attention to during Code Review
HTML Structure and Semantics
- Semantic Elements: Ensure that the code uses semantic HTML elements (e.g.,
<header>
,<nav>
,<article>
,<section>
,<footer>
) to meaningfully structure content. - Common Mistake: Using
<div>
elements when a semantic element would be more appropriate. - PR Comments: "Could you use a
<section>
element element here instead of<div>
? It would be semantically more fitting." - Accessibility: Check, for instance, whether all images have `alt`` attributes and if form elements are labeled correctly.
- Hierarchy: The structure should be clear and understandable, using tags like
<h1>
,<h2>
,<h3>
etc. in the correct order.
Code Quality
DRY (Don't Repeat Yourself)
- Look for repetitive code or similar patterns that could be consolidated into functions or components.
Logic, Flow, and Clarity
- Clear naming of functions, components, and variables.
- Avoid overly complex or nested logics that are hard to understand.
Clean Code
Naming Functions
Names should be descriptive and not too general or too specific.
🚫
calc
Naming Components
Component names should clearly reflect the content or purpose of the component.
🚫
ProfileComp
Naming Variables
Specific and clear naming; avoid generic names.
🚫
array
, list
Comments
🚫 Commented out code that repeat the code.
// This function adds two numbers
function sum(a, b) {
return a + b;
}
Conditional Expressions
🚫 Complicated and hard-to-understand conditions.
if (
(user &&
user.status === "logged in" &&
user.accessLevel.indexOf("premium") > -1) ||
adminOverride
) {
//...
}
Consistency
- The coding style is consistent throughout the project.
Project Structure
- Check if the project structure is consistent, logical, and easily understandable.
- Ensure all files and folders are sensibly named and organized.
Giving Feedback
- Be Constructive: Provide feedback in a positive and constructive manner. Avoid negative or absolute statements.
- Explain the Reason: When you suggest a change, explain the reason behind it.
- Ask, Don't Order: Use questions to stimulate the author's thinking rather than directly telling them what to do.