From c355d3a3d37067eb4ce93d6e57eb76b96000d948 Mon Sep 17 00:00:00 2001 From: Mikayla Billings-Alston Date: Fri, 22 Sep 2023 14:04:15 -0700 Subject: [PATCH 1/3] WIP --- .cspell.json | 3 +- .../5.5.7-code-review.md | 85 +++++++++++++++++++ docs/README.md | 13 +-- docs/_sidebar.md | 1 + 4 files changed, 95 insertions(+), 7 deletions(-) create mode 100644 docs/5-software-development-practices/5.5.7-code-review.md diff --git a/.cspell.json b/.cspell.json index c6145765..9960a71c 100644 --- a/.cspell.json +++ b/.cspell.json @@ -9,8 +9,8 @@ "Armon", "AWSCLIV", "azurerm", - "blaa", "Bento", + "blaa", "boto", "Brolliar", "burnup", @@ -18,6 +18,7 @@ "cfssljson", "cifs", "CNCF", + "Codecademy", "Codecov", "dbservers", "devel", diff --git a/docs/5-software-development-practices/5.5.7-code-review.md b/docs/5-software-development-practices/5.5.7-code-review.md new file mode 100644 index 00000000..b1a7822e --- /dev/null +++ b/docs/5-software-development-practices/5.5.7-code-review.md @@ -0,0 +1,85 @@ +--- +docs/5-software-development-practices/5.5.7-code-review.md: + category: Software Quality + estReadingMinutes: 10 +--- + +# Code Review + +Automated tools are helpful in ensuring code quality, but these tools can only get you so far. When you're developing in a team, one of the biggest assets you have to ensure the quality of the code entering your codebase is each other. + +Whenever you're attempting to add code to a codebase, chances are you'll need to have a peer review your code. Oftentimes a code review will be required to merge a pull request into your main branch. Receiving feedback on your code can feel intimidating, especially if a group or team doesn't have any agreed upon guidelines for code reviews. + +## General Etiquette + +Code review guidelines and procedures will likely look different from organization to organization, but there are some general guidelines everyone should follow when engaging in code reviews, both from the perspective of the reviewer and the developer. + +### Everyone +- **Be respectful in your comments.** There are humans on both sides of the screen. +- **Have good intentions.** The reviewer and developer should both be going into the process with the intention of introducing quality code into the codebase. + +### Developer +- **Separate yourself from your work.** Remember that criticism of your code is not critisms of you. +- **Be open to feedback.** This might seem obvious, but when you're asking for a code review, be ready to take into account any feedback you get rather than just expecting a rubber stamp from the reviewer. + +### Reviewer +- **Comment only on the code, not on the developer.** Again, keep your comments respectful and releveant. Avoid statements about the developer, and keep your statements about the code. +- **Consider adding positive feedback.** It can help build rapport and trust between you and the developer if you point out positive things you see in someone's code rather than just negative things. + +## Before the Review + +When you're developing code, it's good to keep in mind that someone will need to review it. Try to write your code in such a way that it can be understood by others. + +If you want to make a pull request, consider if your code is ready to be merged into the codebase. Are there testing requirements for this repository, and does your code meet these? Is your code complete, or is it a work in progress? If you want input but your code isn't quite ready, consider making a [draft pull request](https://github.blog/2019-02-14-introducing-draft-pull-requests/). + +If you've decided it's the right time to raise a pull request, it's worth taking a look at some things you can do before the review to make the review easier both for yourself and the reviewer. If it's not already integrated into the repository, you could run a linter on your code to catch small, easy to fix things like styling, or look into other automated tools that assist with code review. + +When you're raising a pull request, you should leverage the description field not only to document what changes you're making, but also to help the reviewer understand why this pull request is being raised and what it's supposed to do. If this pull request addresses a ticket, you should reference that ticket to give additional context. + +## Finding a Reviewer + +Making a pull request doesn't necessarily guarantee someone will see it. Depending on the organization you're working in, there may be different ways of notifying potential reviewers, like through your typical communication platform (Slack, Microsoft Teams, etc) or by requesting reviews from specific users through Github. + +When seeking out a reviewer, you should try to get a review from someone who is at least generally familiar with the project you're contributing to. That being said, you should still try to write understandable code and add a through description. + +## Reviewing Code + +When you're reviewing code, your goal is to help find any potential defects in the code and help the developer make their code be the best it can be. This can feel pretty intimidating if you haven't done it before, but there are some things you can do to try to give structure to your review. + +- **Consider writing a checklist.** You could write a checklist of things you're looking for when you're going into a code review. This list could include things like whether or not the code is secure, if it accomplishes the task it says it's meant to, etc. + +- **Think about if now is the right time to do a review.** Sometimes you might not be in the right headspace to do a code review. Doing a code review while your tired or frustrated could result in you missing bugs/errors in the code, or making less than polite comments. + +- **Denote nitpicks.** If you have an opinion about the way a piece of code is written, feel free to share that opinion, but make sure you make it clear when you're presenting an alternative way to do something and when you're commenting on a blocking issue. + +## Specific Guidelines + +If you're working in a small team without dedicated code review guidelines, consider taking some time to formalize your groups approach. Some good questions to ask to guide what you want your process to look like could include: + +- How should a developer seek out a reviewer? + +- Are you going to integrate any automated tools to assist with the code review process? + +- How should a reviewer denote blocking issues and nitpicks? + +- Should reviewers put forth particular code suggestions? + +- Who's responsibility is it to make changes to PRs that need small fixes? + +If you're working in an organization that already has code review customs put into place, take some time to familiarize yourself with that particular process before reviewing code or asking for a code review. + +## Examples and Other Resources + +[How to Write the Perfect Pull Request - Github](https://github.blog/2015-01-21-how-to-write-the-perfect-pull-request/) + +[Code Review Developer Guide - Google](https://google.github.io/eng-practices/review/) + +[5 Code Review Best Practices - Atlassian](https://www.atlassian.com/blog/add-ons/code-review-best-practices) + +[How To Review Someone Else’s Code: Tips and Best Practices - Codecademy](https://www.codecademy.com/resources/blog/code-review-best-practices/) + +# Deliverables + +- Assume you're reviewing some code, and you find a line that you would have written differently. Give some examples of a good way to comment on this, and a bad way to comment on this. + +- Say you find a potential bug in the code. Give some examples of a good way to comment on this, and a bad way to comment on this. \ No newline at end of file diff --git a/docs/README.md b/docs/README.md index dd0f6f1a..3c1a3501 100644 --- a/docs/README.md +++ b/docs/README.md @@ -454,19 +454,17 @@ docs/4-cloud-computing/4.3.6-aks.md: - Azure Kubernetes Service (AKS) - Azure Container Registry (ACR) - Node.js - docs/4-cloud-computing/4.3.7-app-service.md: category: Cloud Computing estReadingMinutes: 10 exercises: - - - name: Deploy Simple Web Application to App Service + - name: Deploy Simple Web Application to App Service description: Create A Web App and Scale and Monitor estMinutes: 240 technologies: - - Azure - - Azure App Service - - Azure CLI + - Azure + - Azure App Service + - Azure CLI docs/5-software-development-practices/5.1-overview.md: category: Agile Development estReadingMinutes: 90 @@ -580,6 +578,9 @@ docs/5-software-development-practices/5.5.6-sonarqube.md: technologies: - SonarQube - GitHub Actions +docs/5-software-development-practices/5.5.7-code-review.md: + category: Software Quality + estReadingMinutes: 10 docs/5-software-development-practices/5.6-hello-devops.md: category: Agile Development estReadingMinutes: 5 diff --git a/docs/_sidebar.md b/docs/_sidebar.md index 0d714d3c..8c330dc9 100644 --- a/docs/_sidebar.md +++ b/docs/_sidebar.md @@ -91,6 +91,7 @@ - [5.5.4 - Code Coverage](5-software-development-practices/5.5.4-code-coverage.md) - [5.5.5 - Test Automation](5-software-development-practices/5.5.5-test-automation.md) - [5.5.6 - SonarQube](5-software-development-practices/5.5.6-sonarqube.md) + - [5.5.7 - Code Review](5-software-development-practices/5.5.7-code-review.md) - [5.6 - Hello DevOps](5-software-development-practices/5.6-hello-devops.md) - **Chapter 6** From 8fa75cbc99d913b755560e444e109533233e79fc Mon Sep 17 00:00:00 2001 From: Mikayla Billings-Alston Date: Fri, 22 Sep 2023 14:20:53 -0700 Subject: [PATCH 2/3] Fixing linting errors --- .../5.5.7-code-review.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/5-software-development-practices/5.5.7-code-review.md b/docs/5-software-development-practices/5.5.7-code-review.md index b1a7822e..a82d4203 100644 --- a/docs/5-software-development-practices/5.5.7-code-review.md +++ b/docs/5-software-development-practices/5.5.7-code-review.md @@ -6,23 +6,26 @@ docs/5-software-development-practices/5.5.7-code-review.md: # Code Review -Automated tools are helpful in ensuring code quality, but these tools can only get you so far. When you're developing in a team, one of the biggest assets you have to ensure the quality of the code entering your codebase is each other. +Automated tools are helpful in ensuring code quality, but these tools can only get you so far. When you're developing in a team, one of the biggest assets you have to ensure the quality of the code entering your codebase is each other. -Whenever you're attempting to add code to a codebase, chances are you'll need to have a peer review your code. Oftentimes a code review will be required to merge a pull request into your main branch. Receiving feedback on your code can feel intimidating, especially if a group or team doesn't have any agreed upon guidelines for code reviews. +Whenever you're attempting to add code to a codebase, chances are you'll need to have a peer review your code. Oftentimes a code review will be required to merge a pull request into your main branch. Receiving feedback on your code can feel intimidating, especially if a group or team doesn't have any agreed upon guidelines for code reviews. ## General Etiquette Code review guidelines and procedures will likely look different from organization to organization, but there are some general guidelines everyone should follow when engaging in code reviews, both from the perspective of the reviewer and the developer. ### Everyone + - **Be respectful in your comments.** There are humans on both sides of the screen. - **Have good intentions.** The reviewer and developer should both be going into the process with the intention of introducing quality code into the codebase. ### Developer + - **Separate yourself from your work.** Remember that criticism of your code is not critisms of you. - **Be open to feedback.** This might seem obvious, but when you're asking for a code review, be ready to take into account any feedback you get rather than just expecting a rubber stamp from the reviewer. ### Reviewer + - **Comment only on the code, not on the developer.** Again, keep your comments respectful and releveant. Avoid statements about the developer, and keep your statements about the code. - **Consider adding positive feedback.** It can help build rapport and trust between you and the developer if you point out positive things you see in someone's code rather than just negative things. @@ -32,7 +35,7 @@ When you're developing code, it's good to keep in mind that someone will need to If you want to make a pull request, consider if your code is ready to be merged into the codebase. Are there testing requirements for this repository, and does your code meet these? Is your code complete, or is it a work in progress? If you want input but your code isn't quite ready, consider making a [draft pull request](https://github.blog/2019-02-14-introducing-draft-pull-requests/). -If you've decided it's the right time to raise a pull request, it's worth taking a look at some things you can do before the review to make the review easier both for yourself and the reviewer. If it's not already integrated into the repository, you could run a linter on your code to catch small, easy to fix things like styling, or look into other automated tools that assist with code review. +If you've decided it's the right time to raise a pull request, it's worth taking a look at some things you can do before the review to make the review easier both for yourself and the reviewer. If it's not already integrated into the repository, you could run a linter on your code to catch small, easy to fix things like styling, or look into other automated tools that assist with code review. When you're raising a pull request, you should leverage the description field not only to document what changes you're making, but also to help the reviewer understand why this pull request is being raised and what it's supposed to do. If this pull request addresses a ticket, you should reference that ticket to give additional context. @@ -82,4 +85,4 @@ If you're working in an organization that already has code review customs put in - Assume you're reviewing some code, and you find a line that you would have written differently. Give some examples of a good way to comment on this, and a bad way to comment on this. -- Say you find a potential bug in the code. Give some examples of a good way to comment on this, and a bad way to comment on this. \ No newline at end of file +- Say you find a potential bug in the code. Give some examples of a good way to comment on this, and a bad way to comment on this. From 1277979f56de287cdb0ca726c6a55bf546ef422c Mon Sep 17 00:00:00 2001 From: Mikayla Billings-Alston Date: Tue, 26 Sep 2023 14:47:35 -0700 Subject: [PATCH 3/3] Updating phrasing --- docs/5-software-development-practices/5.5.7-code-review.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/5-software-development-practices/5.5.7-code-review.md b/docs/5-software-development-practices/5.5.7-code-review.md index a82d4203..3a0fcd15 100644 --- a/docs/5-software-development-practices/5.5.7-code-review.md +++ b/docs/5-software-development-practices/5.5.7-code-review.md @@ -17,7 +17,7 @@ Code review guidelines and procedures will likely look different from organizati ### Everyone - **Be respectful in your comments.** There are humans on both sides of the screen. -- **Have good intentions.** The reviewer and developer should both be going into the process with the intention of introducing quality code into the codebase. +- **Have good intentions.** The reviewer and developer should both be going into the process with the intention of ensuring and maintaining quality code in the codebase. ### Developer @@ -33,7 +33,7 @@ Code review guidelines and procedures will likely look different from organizati When you're developing code, it's good to keep in mind that someone will need to review it. Try to write your code in such a way that it can be understood by others. -If you want to make a pull request, consider if your code is ready to be merged into the codebase. Are there testing requirements for this repository, and does your code meet these? Is your code complete, or is it a work in progress? If you want input but your code isn't quite ready, consider making a [draft pull request](https://github.blog/2019-02-14-introducing-draft-pull-requests/). +Before making a pull request, consider if your code is ready to be merged into the codebase. Are there testing requirements for this repository, and does your code meet these? Is your code complete, or is it a work in progress? If you want input but your code isn't quite ready, consider making a [draft pull request](https://github.blog/2019-02-14-introducing-draft-pull-requests/). If you've decided it's the right time to raise a pull request, it's worth taking a look at some things you can do before the review to make the review easier both for yourself and the reviewer. If it's not already integrated into the repository, you could run a linter on your code to catch small, easy to fix things like styling, or look into other automated tools that assist with code review.