diff --git a/_episodes/11-software-project.md b/_episodes/11-software-project.md index 986c5d742..0516ecd8b 100644 --- a/_episodes/11-software-project.md +++ b/_episodes/11-software-project.md @@ -54,8 +54,7 @@ to fix and build on top of the existing code during the course. ## Downloading Our Software Project -To start working on the project, you will first -create a copy of the software project template repository +To start working on the project, you will first create a fork of the software project repository from GitHub within your own GitHub account and then obtain a local copy of that project (from your GitHub) on your machine. @@ -63,33 +62,37 @@ and then obtain a local copy of that project (from your GitHub) on your machine. and that you have set up your SSH key pair for authentication with GitHub, as explained in [Setup](../setup.html#secure-access-to-github-using-git-from-command-line). 2. Log into your GitHub account. -3. Go to the [software project template repository](https://github.com/carpentries-incubator/python-intermediate-inflammation) +3. Go to the [software project repository](https://github.com/carpentries-incubator/python-intermediate-inflammation) in GitHub. - ![Software project template repository in GitHub](../fig/template-repository.png){: .image-with-shadow width="800px" } + ![Software project fork repository in GitHub](../fig/github-fork-repository.png){: .image-with-shadow width="900px" } -4. Click the `Use this template` button - towards the top right of the template repository's GitHub page to create - a **copy** of the repository under your GitHub account - (you will need to be signed into GitHub to see the `Use this template` button). - Note that each participant is creating their own copy to work on. - Also, we are not forking the directory but creating a copy - (remember - you can have only one *fork* but can have multiple *copies* of a repository in GitHub). +4. Click the `Fork` button + towards the top right of the repository's GitHub page to create + a **fork** of the repository under your GitHub account + (you will need to be signed into GitHub for the `Fork` button to work). + Note that each participant is creating their own fork to work on. + Also, we are not copying from a template but creating a fork + (remember you can have only one *fork* + but can have multiple *copies* of a repository in GitHub). 5. Make sure to select your personal account and set the name of the project to `python-intermediate-inflammation` (you can call it anything you like, but it may be easier for future group exercises if everyone uses the same name). - Also set the new repository's visibility to 'Public' - - so it can be seen by others and by third-party Continuous Integration (CI) services - (to be covered later on in the course). + Ensure that you **uncheck** the `Copy the main branch only` button. + This will guarantee we get some other branches needed for later exercises, + but for the moment you can ignore them. - ![Making a copy of the software project template repository in GitHub](../fig/copy-template-repository.png){: .image-with-shadow width="600px" } + ![Making a fork of the software project repository in GitHub](../fig/github-fork-repository-confirm.png){: .image-with-shadow width="600px" } -6. Click the `Create repository from template` button - and wait for GitHub to import the copy of the repository under your account. -7. Locate the copied repository under your own GitHub account. +6. Click the `Create fork` button + and wait for GitHub to create the forked copy of the repository under your account. +7. Locate the forked repository under your own GitHub account. + You should be taken there automatically after confirming the fork operation, + but if not, you can click your username top left to be taken to your user page, + and then select the `Repositories` tab, where you can search for this new fork. - ![View of the own copy of the software template repository in GitHub](../fig/own-template-repository.png){: .image-with-shadow width="800px" } + ![View of your own fork of the software repository in GitHub](../fig/github-forked-repository-own.png){: .image-with-shadow width="900px" } > ## Exercise: Obtain the Software Project Locally > Using the command line, clone the copied repository @@ -97,7 +100,7 @@ and then obtain a local copy of that project (from your GitHub) on your machine. > Which command(s) would you use to get a detailed list of contents of the directory you have just cloned? > > ## Solution > > 1. Find the SSH URL of the software project repository to clone from your GitHub account. -> > Make sure you do not clone the original template repository but rather your own copy, +> > Make sure you do not clone the original repository but rather your own fork, > > as you should be able to push commits to it later on. > > Also make sure you select the **SSH tab** and not the **HTTPS** one - > > you'll be able to clone with HTTPS, but not to send your changes back to GitHub! @@ -114,7 +117,7 @@ and then obtain a local copy of that project (from your GitHub) on your machine. > > $ git clone git@github.com:/python-intermediate-inflammation.git > > ~~~ > > {: .language-bash} -> > Make sure you are cloning your copy of the software project and not the template repository. +> > Make sure you are cloning your fork of the software project and not the original repository. > > > > 4. Navigate into the cloned repository folder in your command line with: > > ~~~ diff --git a/_episodes/41-code-review.md b/_episodes/41-code-review.md index 0637666f4..4b4ef6f48 100644 --- a/_episodes/41-code-review.md +++ b/_episodes/41-code-review.md @@ -1,7 +1,7 @@ --- title: "Developing Software In a Team: Code Review" -teaching: 15 -exercises: 30 +teaching: 30 +exercises: 40 questions: - "How do we develop software in a team?" - "What is code review and how it can improve the quality of code?" @@ -25,84 +25,52 @@ Software is often designed and built as part of a team, so in this episode we'll be looking at how to manage the process of team software development and improve our code by engaging in code review process with other team members. -> ## Collaborative Code Development Models -> The way your team provides contributions to the shared codebase depends on -> the type of development model you use in your project. -> Two commonly used models are: -> -> * Fork and pull model -> * Shared repository model -> -> **Fork and Pull Model** -> -> In this model, anyone can **fork** an existing repository -> (to create their copy of the project linked to the source) -> and push changes to their personal fork. -> A contributor can work independently on their own fork as they do not need -> permissions on the source repository to push modifications to a fork they own. -> The changes from contributors can then be **pulled** into the source repository -> by the project maintainer on request and after a code review process. -> This model is popular with open source projects as it -> reduces the start up costs for new contributors -> and allows them to work independently without upfront coordination -> with source project maintainers. -> So, for example, you may use this model when you are an external collaborator on a project -> rather than a core team member. -> -> **Shared Repository Model** -> -> In this model, collaborators are granted push access to a single shared code repository. -> By default, collaborators have write access to the main branch. -> However, it is best practice to create feature branches for new developments, -> and protect the main branch. See the extra on [protecting the main branch](../protect-main-branch) -> for how to do this. -> While it requires more upfront coordination, it is easier to share each others -> work, so it works well for more stable teams. -> This model is more prevalent with teams and organizations collaborating on private projects. -{: .callout} - -Regardless of the collaborative code development model you and your collaborators use - -code reviews are one of the widely accepted best practices for software development in teams -and something you should adopt in your development process too. +## Collaborative Code Development Models +The way a team provides contributions to a shared codebase depends on +the type of development model used in a project. +Two commonly used models are described below. + +### Fork and Pull Model +In the **fork and pull** model, anyone can **fork** an existing repository +(to create their copy of the project linked to the source) +and push changes to their personal fork. +A contributor can work independently on their own fork as they do not need +permissions on the source repository to push modifications to a fork they own. +The changes from contributors can then be **pulled** into the source repository +by the project maintainer on request and after a code review process. +This model is popular with open source projects as it +reduces the start up costs for new contributors +and allows them to work independently without upfront coordination +with source project maintainers. +So, for example, you may use this model when you are an external collaborator on a project +rather than a core team member. + +### Shared Repository Model + +In the **shared repository model**, collaborators are granted push access to a single shared code repository. +By default, collaborators have write access to the main branch. +However, it is best practice to create feature branches for new developments, +and protect the main branch. See the [extra episode on protecting the main branch](../protect-main-branch) +on how to do this. +While it requires more upfront coordination, it is easier to share each others +work, so it works well for more stable teams. +This model is more prevalent with teams and organisations collaborating on private projects. ## Code Review -[Code review][code-review] is a software quality assurance practice -where one or several people from the team (different from the code's author) -check the software by viewing parts of its source code. - -> ## Group Exercise: Advantages of Code Review -> Discuss as a group: what do you think are the reasons behind, and advantages of, code review? -> > ## Solution -> > The purposes of code review include: -> > - improving internal code readability, understandability, quality and maintainability -> > - checking for coding standards compliance, code uniformity and consistency -> > - checking for test coverage and detecting bugs and code defects early -> > - detecting performance problems and identifying code optimisation points -> > - finding alternative/better solutions. -> > -> > An effective code review prevents errors from creeping into your software -> > by improving code quality at an early stage of the software development process. -> > It helps with learning, i.e. sharing knowledge about the codebase, -> > solution approaches, -> > expectations regarding quality, -> > coding standards, etc. -> > Developers use code review feedback from more senior developers -> > to improve their own coding practices and expertise. -> > Finally, it helps increase the sense of collective code ownership and responsibility, -> > which in turn helps increase the "bus factor" -> > and reduce the risk resulting from information and capabilities -> > being held by a single person "responsible" for a certain part of the codebase -> > and not being shared among team members. -> {: .solution} -{: .challenge} +Regardless of the collaborative code development model you and your collaborators use, +[code reviews][code-review] are one of the widely accepted best practices for software development in teams +and something you should adopt in your development process too. -Code review is one of the most useful team code development practices - -someone checks your design or code for errors, they get to learn from your solution, +Code review is a software quality assurance practice +where one or several people from the team (different from the code's author) +check the software by viewing parts of its source code when code changes are introduced. +It is one of the most useful team code development practices - +someone checks your design or code for errors and gets to learn from your solution; having to explain code to someone else clarifies -your rationale and design decisions in your mind too, -and collaboration helps to improve the overall team software development process. -It is universally applicable throughout the software development cycle - +your rationale and design decisions in your mind too. + +Code review is universally applicable throughout the software development cycle - from design to development to maintenance. According to Michael Fagan, the author of the [code inspection technique](https://en.wikipedia.org/wiki/Fagan_inspection), @@ -118,48 +86,21 @@ as close as possible to the point where they were introduced. There are several **code review techniques** with various degree of formality and the use of a technical infrastructure, including: -- **Over-the-shoulder code review** - is the most common and informal of code review techniques and involves - one or more team members standing over the code author's shoulder - while the author walks the reviewers through a set of code changes. -- **Email pass-around code review** - is another form of lightweight code review where the code author - packages up a set of changes and files and sends them over to reviewers via email. - Reviewers examine the files and differences against the code base, - ask questions and discuss with the author and other developers, - and suggest changes over email. - The difficult part of this process is the manual collection the files under review - and noting differences. -- **Pair programming** - is a code development process that incorporates continuous code review - - two developers sit together at a computer, - but only one of them actively codes whereas the other provides real-time feedback. - It is a great way to inspect new code and train developers, - especially if an experienced team member walks a younger developer through the new code, - providing explanations and suggestions through a conversation. - It is conducted in-person and synchronously but it can be time-consuming - as the reviewer cannot do any other work during the pair programming period. -- **Fagan code inspection** - is a formal and heavyweight process of finding defects in specifications or designs - during various phases of the software development process. - There are several roles taken by different team members in a Fagan inspection - and each inspection is a formal 7-step process with a predefined entry and exit criteria. - See [Fagan inspection](https://en.wikipedia.org/wiki/Fagan_inspection) - for full details on this method. -- **Tool-assisted code review** - process uses a specialised tool to facilitate the process of code review, - which typically helps with the following tasks: - (1) collecting and displaying the updated files and highlighting what has changed, - (2) facilitating a conversation between team members (reviewers and developers), and - (3) allowing code administrators and product managers - a certain control and overview of the code development workflow. - Modern tools may provide a handful of other functionalities too, such as metrics - (e.g. inspection rate, defect rate, defect density). - -Each of the above techniques have their pros and cons and varying degrees practicality - -it is up to the team to decide which ones are most suitable for the project and when to use them. -We will have a look at the **tool-assisted code review process** -using GitHub's built-in code review tool - **pull requests**. + - [Over-the-shoulder code review](https://about.gitlab.com/topics/version-control/what-is-code-review/#Over-the-shoulder%20reviews) - + one developer talks the other developer through the code changes whilst sat at the same machine. + - [Pair programming](https://about.gitlab.com/topics/version-control/what-is-code-review/#Pair%20programming) - + two developers work on the code at the same time where one of them actively codes whereas the other provides real-time feedback. + - [Formal code inspection](https://en.wikipedia.org/wiki/Fagan_inspection) - + up to 6 partipants go through a formalised process to inspect the code specifications or design for defects. + - [Tool assisted code review](https://about.gitlab.com/topics/version-control/what-is-code-review/#Tool-assisted%20reviews) - + developers use tools such as GitHub to review the code independently and give feedback. + +You can read more about these techniques in the ["Five Types of Review" section](https://www.khoury.northeastern.edu/home/lieber/courses/cs4500/f07/lectures/code-review-types.pdf) of the ["Best Kept Secrets of Peer Code Review" eBook](https://www.yumpu.com/en/document/view/19324443/best-kept-secrets-of-peer-code-review-pdf-smartbear). + +It is worth trying multiple code review techniques to see what works +best for you and your team. +We will have a look at the **tool-assisted code review process**, which is likely to be the most effective and efficient. +We will use GitHub's built-in code review tool - **pull requests**, or PRs. It is a lightweight tool, included with GitHub's core service for free and has gained popularity within the software development community in recent years. @@ -172,7 +113,7 @@ and that your code is ready for review. Once a pull request is opened, you can discuss and review the potential changes with others on the team and add follow-up commits based on the feedback -before your changes are merged from your feature branch into the `develop` branch. +before your changes are merged into the development branch. The name 'pull request' suggests you are **requesting** the codebase moderators to **pull** your changes into the codebase. @@ -188,11 +129,15 @@ create a pull request to bring the changes back to the branch that you started f In this context, the branch from which you branched off to do your work and where the changes should be applied back to is called the **base branch**, -while the feature branch that contains changes you would like to be applied is the **head branch**. +while the feature branch that contains changes you would like to be applied is the **compare branch**. How you create your feature branches and open pull requests in GitHub will depend on your collaborative code development model: +- In the fork and pull model, + where you do not have write permissions to the source repository, + you need to fork the repository first + before you create a feature branch (in your fork) to base your pull request on. - In the shared repository model, in order to create a feature branch and open a pull request based on it you must have write access to the source repository or, @@ -200,10 +145,6 @@ your collaborative code development model: you must be a member of the organisation that owns the repository. Once you have access to the repository, you proceed to create a feature branch on that repository directly. -- In the fork and pull model, - where you do not have write permissions to the source repository, - you need to fork the repository first - before you create a feature branch (in your fork) to base your pull request on. In both development models, it is recommended to create a feature branch for your work and the subsequent pull request, @@ -214,286 +155,493 @@ and update your proposed changes within a self-contained bundle. The only difference in creating a pull request between the two models is how you create the feature branch. In either model, once you are ready to merge your changes in - -you will need to specify the base branch and the head branch. - -## Code Review and Pull Requests In Action +you will need to specify the base branch and the compare branch. Let's see this in action - -you and your fellow learners are going to be organised in small teams -and assume to be collaborating in the shared repository model. -You will be added as a collaborator to another team member's repository -(which becomes the shared repository in this context) -and, likewise, you will add other team members as collaborators on your repository. -You can form teams of two and work on each other's repositories. -If there are 3 members in your group you can go in a round robin fashion -(the first team member does a pull request on the second member's repository -and receives a pull request on their repository from the third team member). -If you are going through the material on your own and do not have a collaborator, -you can do pull requests on your own repository from one to another branch. - -Recall [solution requirements SR1.1.1 and SR1.2.1](../31-software-requirements/index.html#solution-requirements) +you are going to act as a reviewer on a change to the codebase raised by a +fictional colleague on one of your fellow learner's repository. +Once this is done, you will then take on the role of the fictional colleague +and respond to the review on your repository. +If you are completing this course by yourself, you can raise the review on +your own repository, review it there and finally respond to your own review +comments. This is actually a very sensible thing to do in general - looking +at your own code in a review window will allow you to spot mistakes you +have not seen before. + +Here is an outline of the process of a tool-assisted code review. + +{% comment %} +```mermaid +%% Must disable useMaxWidth to have the diagram centre +%%{init: { 'sequence': {'useMaxWidth':false} } }%% +sequenceDiagram + participant A as Author + participant R as Reviewer + A->>A: Write some code + A->>R: Raise a pull request + R->>R: Add comments to a review + R->>A: Submit a review + loop Until approved + A->>R: Address or respond to review comments + R->>A: Clarify or resolve comments + end + R->>A: Approve pull request + A->>A: Merge pull request +``` +Generated with Mermaid tool, diagram available at: https://mermaid.live/edit#pako:eNptUrtuwzAM_BVCgOEl_QEPAYx29eKgyOKFsZhYgF6VKLeBkX-vXDvNo5UWSXfHI0VOoneSRCWKApoUGaSKeNAEKVKDX3sleQB2MOBIwANlHE8BDfRkOVBni2JSVnEFE5SRPhLZnsp8K-8ClNURdaQL5F0Unb3y3pZYnYW8PAZWvfJoGWrACHXiwYW_YDuDLY2KPmmF65fttq5gHxQTRGcI5qpuWFtBiyoSIPikNYQ5gcgLoV0ItZRZZUyuK84VY2bNHjdSdtilg1H8hGnnPLxbVhrQ--BGkgtwZ5-jB4oRXMjS6J2Vs8cS5df2plrtXjUGdTyvKqdHeuKSlQ_51Yv_P1WuX9RQOD3CYiMMBYNK5imYZnIncqMNdaLKR0lHTJo70dlLpmJitzvbXlQcEm1E8hL52sjrI0nFLjTLYP3M1-Ub_pDLrQ +{% endcomment %} +![Code review process sequence](../fig/code-review-sequence-diagram.svg){: .image-with-shadow width="600px"} + +Recall [solution requirements SR1.1.1](../31-software-requirements/index.html#solution-requirements) from an earlier episode. -Your team member has implemented one of them according to the specification -(let's call it `feature-x`) -but tests are still missing. -You are now tasked with implementing tests on top of that existing implementation -to make sure the new feature indeed satisfies the requirements. -You will propose changes to their repository -(the shared repository in this context) -via pull request (acting as the code author) -and engage in code review with your team member (acting as a code reviewer). -Similarly, you will receive a pull request on your repository from another team member, -in which case the roles will be reversed. -The following diagram depicts the branches that you should have in the repository. - -![Branches for a feature and its tests](../fig/exercise-feature-branch.svg){: .image-with-shadow width="800px"} -

-Adapted from Git Tutorial by sillevl (Creative Commons Attribution 4.0 International License) -

- -To achieve this, the following steps are needed. - -#### Step 1: Adding Collaborators to a Shared Repository - -You need to add the other team member(s) as collaborator(s) on your repository -to enable them to create branches and pull requests. -To do so, each repository owner needs to: - -1. Head over to Settings section of your software project's repository in GitHub. - ![Accessing settings for a repository in GitHub](../fig/github-settings.png){: .image-with-shadow width="900px"} -2. Select the **vertical** tab 'Collaborators' from the left and click the 'Add people' button. - ![Managing access to a repository in GitHub](../fig/github-manage-access.png){: .image-with-shadow width="900px"} -3. Add your collaborator(s) by their GitHub username(s), full name(s) or email address(es). - ![Adding collaborators to a repository in GitHub](../fig/github-add-collaborators.png){: .image-with-shadow width="900px"} -4. Collaborator(s) will be notified of your invitation to join your repository - based on their notification preferences. -5. Once they accept the invitation, they will have the collaborator-level access to your repository - and will show up in the list of your collaborators. - -See the full details on -[collaborator permissions for personal repositories](https://docs.github.com/en/account-and-profile/setting-up-and-managing-your-github-user-account/managing-user-account-settings/permission-levels-for-a-user-account-repository) -to understand what collaborators will be able to do within your repository. -Note that repositories owned by an organisation have a -[more granular access control](https://docs.github.com/en/get-started/learning-about-github/access-permissions-on-github) -compared to that of personal repositories. - -#### Step 2: Preparing Your Local Environment for a Pull Request - -1. Obtain the GitHub URL of the shared repository you will be working on and clone it locally - (make sure you do it outside your software repository's folder you have been working on so far). - This will create a copy of the repository locally on your machine - along with all of its (remote) branches. - ~~~ - $ git clone - $ cd - ~~~ - {: .language-bash} -2. Check with the repository owner (your team member) - which feature (SR1.1.1 or SR1.2.1) they implemented in the - [previous exercise](/32-software-design/index.html#implement-requirements) - and what is the name of the branch they worked on. - Let's assume the name of the branch was `feature-x` - (you should amend the branch name for your case accordingly). -3. Your task is to add tests for the code on `feature-x` branch. - You should do so on a separate branch called `feature-x-tests`, - which will branch off `feature-x`. - This is to enable you later on to create a pull request - from your `feature-x-tests` branch with your changes - that can then easily be reviewed and compared with `feature-x` - by the team member who created it. - - To do so, branch off a new local branch `feature-x-tests` from the remote `feature-x` branch - (making sure you use the branch names that match your case). - Also note that, while we say "remote" branch `feature-x` - - you have actually obtained it locally on your machine when you cloned the remote repository. - ~~~ - $ git switch -c feature-x-tests origin/feature-x - ~~~ - {: .language-bash} - - You are now located in the new (local) `feature-x-tests` branch - and are ready to start adding your code. - -#### Step 3: Adding New Code - -> ## Exercise: Implement Tests for the New Feature -> Look back at the -> [solution requirements](/31-software-requirements/index.html#solution-requirements) -> (SR1.1.1 or SR1.2.1) -> for the feature that was implemented in your shared repository. -> Implement tests against the appropriate specification in your local feature branch. -> -> *Note: Try not to not fall into the trap of -> writing the tests to test the existing code/implementation - -> you should write the tests to make sure the code satisfies the requirements -> regardless of the actual implementation. -> You can treat the implementation as a -> [black box](https://en.wikipedia.org/wiki/Black-box_testing) - -> a typical approach to software testing - -> as a way to make sure it is properly tested against its requirements -> without introducing assumptions into the tests about its implementation.* -{: .challenge} - -> ## Testing Based on Requirements -> Tests should test functionality, -> which stem from the software requirements, -> rather than an implementation. -> Tests can be seen as a reflection of those requirements - -> checking if the requirements are satisfied. -{: .callout} - -Remember to commit your new code to your branch `feature-x-tests`. - -~~~ -$ git add -A -$ git commit -m "Added tests for feature-x." -~~~ -{: .language-bash} - -#### Step 4: Submitting a Pull Request - -When you have finished adding your tests -and committed the changes to your local `feature-x-tests`, -and are ready for the others in the team to review them, -you have to do the following: - -1. Push your local feature branch `feature-x-tests` remotely to the shared repository. - ~~~ - $ git push -u origin feature-x-tests - ~~~ - {: .language-bash} -2. Head over to the remote repository in GitHub - and locate your new (`feature-x-tests`) branch from the dropdown box on the Code tab - (you can search for your branch or use the "View all branches" option). - ![All repository branches in GitHub](../fig/github-branches.png){: .image-with-shadow width="600px"} -3. Open a pull request by clicking "Compare & pull request" button. - ![Submitting a pull request in GitHub](../fig/github-create-pull-request.png){: .image-with-shadow width="900px"} -4. Select the base and the head branch, e.g. `feature-x` and `feature-x-tests`, respectively. +Your fictional colleague has implemented it according to the specification +and pushed it to a branch `feature-std-dev`. +You will turn this branch into a pull request for your fictional colleague on your +repository. +You will then engage in code review for the change (acting as a code reviewer) on +a fellow learner's repository. +Once complete, you will respond to the comments from another team member on the pull request +on your repository (acting as a code author). + +### Raising a Pull Request + +1. Head over to your software repository in GitHub. +2. Navigate to the pull requests tab. +3. Create a new pull request by clicking the green `New pull request` button. + ![GitHub pull requests tab](../fig/github-pull-request-tab.png){: .image-with-shadow width="900px"} +4. Select the base and the compare branch, e.g. `main` and `feature-std-dev`, respectively. Recall that the base branch is where you want your changes to be merged - and the head branch contains your changes. -5. Add a comment describing the nature of the changes, - and then submit the pull request. -6. Repository moderator and other collaborators on the repository (code reviewers) - will be notified of your pull request by GitHub. + and the compare branch contains the changes. +5. Click `Create pull request` button to open the request. + ![Creating a new pull request.](../fig/github-create-pull-request.png){: .image-with-shadow width="900px"} +6. Add a comment describing the nature of the changes, + and then submit the pull request by clicking the `Create pull request` button (in the new window). +![Submitting a pull request.](../fig/github-submit-pull-request.png){: .image-with-shadow width="900px"} 7. At this point, the code review process is initiated. -You should receive a similar pull request from other team members on your repository. +We will now discuss what to look for in a code review, +before practising it on this fictional change. + +### Reviewing a Pull Request + +Once a review has been raised it is over to the reviewer to review the code +and submit feedback. + +Reviewing code effectively takes practice. +However, here is some guidance on what you should +be looking for when reviewing a piece of code. + +#### Things to Look for in a Code Review + +Start by understanding what the code _should_ do, by reading the specification/user requirements, +the pull request description and talking to the developer. +In this case, understand what [SR1.1.1](../31-software-requirements/index.html#solution-requirements) means. + +Once you're happy, start reading the code (skip the test code for now - we will come back to it later). +You're going to be assessing the code in the following key areas. + +##### Is the proposed code readable? + +* Think about the names of the variables, do they [follow guidelines for good +names?](../15-coding-conventions/index.html#l#naming-conventions) +* Do you understand what conditions in each if statements are for? +* Do the function names match the behavior of the function? + +##### Is the proposed code a minimal change? + +* Does the code reimplement anything that already exists, either +elsewhere in the codebase or in a library you know about? +* Does the code implement something that isn't on the ticket? + +##### Is the structure of the code clear? + +* Do functions do just one thing? +* Is the code using the right level of modularity? +* Is the code consistent with the structure of the rest of the code? + +##### Is there an appropriate and up-to-date documentation for the proposed code? + +* If functionality has changed, has corresponding documentation been +updated? +* If new functions have been added, do they have appropriate +levels of documentation? +* Does the documentation make sense? +* Are there clear and useful comments that explain complex designs +and focus on the "why/because" rather than the "what/how"? + +#### Things Not to Look for in a Code Review + +The overriding priority for reviewing code should be making sure progress is being made - +don't let perfect be the enemy of the good here. +According to [“Best Kept Secrets of Peer Code Review” (Cohen, 2006)](https://www.amazon.co.uk/Best-Kept-Secrets-Peer-Review/dp/1599160676) +the first hour of reviewing code is the most effective, with diminishing returns after that. + +To that end, here are a few things you *shouldn't* be trying to spot when reviewing: +* Linting issues, or anything else that an automated tool can spot - get the Continuous Integration (CI) to do it. +* Bugs - instead make sure there are tests for all cases. +* Issues that pre-date the change - raise separate PRs fixing these issues separately to avoid heading down a rabbit hole. +* Architecture re-writes - try to have design discussions upfront, +or else have a meeting to decide whether the code needs to be rewritten. + +#### Adding a review comment + +1. We will start by having your course mate adding you as a collaborator on their repository. +They can do that from +the `Settings` tab of the repository, then `Collaborators and teams` tab on the left, +then clicking the `Add people` button. +Once they find you by your GitHub name - you will receive an invitation via email to join the +repository as a collaborator. +You will have to do the same for the collaborator doing the review on your repository. + > ## Code Review from External Contributors + > You do not have to be a collaborator on a public repository to do code reviews + > so this step is not strictly necessary. + > We are still asking you to do it now as we will get you working in teams + > for the rest of the course so it may make sense to start setting up your collaborators now. + {: .callout} +![Adding a collaborator in GitHub](../fig/github-add-collaborator.png){: .image-with-shadow width="900px"} +2. Open up the pull request from the GitHub's `Pull Requests` tab on the home page +of your fellow learner's software repository, then head to the `Files changed` tab. +![The files changed tab of a pull request](../fig/github-pull-request-files-changed.png){: .image-with-shadow width="900px"} +3. When you find a line that you want to add a comment to, click on the blue +plus (+) button next to the line. This will bring up a "Write" box to add your comment. +![Adding a review comment to a pull request](../fig/github-pull-request-add-comment.png){: .image-with-shadow width="800px"} +You can also add comments referring to multiple lines by clicking the plus and +dragging down over the relevant lines. +If you want to make a concrete suggestion or a change to the code directly, +such as renaming a variable, you can click the `Add a suggestion` button +(which looks like a document with a plus and a minus in it). +This will populate the comment with the existing code, and you can edit it to be +what you think the code should be. +![Adding a suggestion to a pull request](../fig/github-pull-request-add-suggestion.png){: .image-with-shadow width="800px"} +GitHub will then provide a button for the code author to apply your change directly. +4. Write your comment in the box, and then click `Start review`. +This will save your comment, but not publish it yet. +You can use `Add single comment` button to immediately post a comment. +However, it is best to batch the comments into a single review, so that the author +knows when you have finished adding comments +(and avoid spamming their email with notifications). +5. Continue adding comments in this way, if you have any, using the `Add review comment` button. +on subsequent comments. + +> ## Effective Review Comments +> +> * Make sure your review comments are specific and actionable. +> * Try to be as specific as you can - instead of "this code is unclear" +> instead say "I don't understand what values this variable can hold". +> * Make it clear in the comment if you want something to change as part +> of this pull request. +> * Ideally provide a concrete suggestion (e.g., better variable name). +{: .callout} -#### Step 5: Code Review +> ## Exercise: Review Some Code +> +> Pair up with a colleague from your group/team and go to the pull request your colleague created on their +> project repository. +> If there is an odd number of people in your group, three people can go in a round robin fashion +> (the first team member will review the pull request on the second member’s repository +> and will receive comments on the pull request on their repository from +> the third team member). +> If you are going through the material on your own and do not have a collaborator, +> you can be the reviewer on the pull requests on your own repository. +> +> Review the code, looking for the kinds of problems that we have just discussed. +> There are examples of all four main problem areas in the pull request, +> so try to make at least one suggestion for each area. +> +> Do not submit your review just yet. +> +>> ## Solution +>> +>> Here are some of the things you might have found were wrong with the code. +>> ##### Is the proposed code readable? +>> * Unclear function name `s_dev` - uses an uncommon abbreviation increasing mental load +>> when reading code that calls this function, prefer `standard_deviation`. +>> * Variable `number` not clear what it contains --- prefer business-logic name like `mean` or `mean_of_data` +>> +>> ##### Is the proposed code a minimal change? +>> +>> * Could have used `np.std` to compute standard deviation of data without having to reimplement +>> from scratch. +>> +>> ##### Is the structure of the proposed code clear? +>> * Have the function return the data, rather than having the graph name (a view layer consideration) +>> leak into the model code. +>> +>> ##### Is there an appropriate and up-to-date documentation for the proposed code? +>> +>> * The documentation say it returns the standard deviation, but it actually returns a dictionary containing +>> the standard deviation. +> {: .solution} +{: .challenge} -1. The repository moderator/code reviewers reviews your changes - and provides feedback to you in the form of comments. -2. Respond to their comments and do any subsequent commits, - as requested by reviewers. -3. It may take a few rounds of exchanging comments and discussions until - the team is ready to accept your changes. +#### Making Sure Code is Valid + +The other key thing you want to verify in a code review is that the code is correct and +well tested. +One approach to do this is to build up a list of tests you expect to see +(and the results you'd expect them to have), +and then verify that all these tests are present and correct. + +Start by listing out all the tests you'd expect to see based on the specification. + +As you are going through the code, add to this list with any tests you can think +of, making sure to add tests for: + +* All paths through the code. +* Making each `if` statement be evaluated as `True` and `False`. +* Executing loops with empty, single and multi-element sequences. +* Edge cases that you spot. +* Any circumstances where you are not certain how code would behave. + +Once you have built the list, go through the tests in the pull request. Make sure +the tests test what you expect (so inspect them closely!). + +### Submitting a Review + +Once you have a list of tests you want the author to add, it is time to +submit your review. + +1. To do this, click the `Finish your review` button at the top of the `Files changed` tab. +![Using the finishing your review dialog](../fig/github-submit-pull-request-review.png){: .image-with-shadow width="800px"} +In the comment box, you can add any comments that aren't +associated with a specific line. +For example, you can put the list of tests that you want to see +added here. +2. Next you will need select to one of `Comment`, `Approve` or `Request changes`. +![Using the finishing your review dialog](../fig/github-finish-pull-request-review.png){: .image-with-shadow width="900px"} +* Use `Approve` if you would be happy for the code to +go in with no further changes. +* Use `Request changes` to communicate to the author that +they should address your comments before you will approve it. +* Use `Comment` if you don't want to express a decision on +whether the code should be accepted. For example, if you've been asked +to look at a specific part of the code, or if you are part way through +a review, but wanted to share some comments sooner. +3. Finally, you can press `Submit review` button. +This will publish all the comments you've made as part of the review and +let the author know that the review is complete and it is their +turn for action. + +> ## Exercise: Review the Code for Suitable Tests +> +> Remind yourself of the specification of SR1.1.1 and write a list of +> tests you'd expect to see for this feature. +> Review the code again and expand this list to include any other +> edge cases the code makes you think of. +> Go through the tests in the pull request and work out which tests are present. +> +> Once you are happy, you can submit your review. +> Select `Request changes` to let the author know they need to address your comments. +> +>> ## Solution +>> +>> Your list might include the following: +>> +>> 1. Standard deviation for one patient with multiple observations. +>> 2. Standard deviation for two patients. +>> 3. Graph includes a standard deviation graph. +>> 4. Standard deviation function should raise an error if given empty data. +>> 5. Computing standard deviation where deviation is different from variance. +>> 6. Standard deviation function should give correct result given negative inputs. +>> 7. Function should work with numpy arrays. +>> +>> Looking at the tests in the PR, you might be content that tests for 1, 4 and 7 are present +>> so you would request changes to add tests 2, 3, 5 and 6. +>> +>> In looking at the tests, you may have noticed that the test for numpy arrays is currently +>> spuriously passing as it does not use the return value from the function in the assert. +>> +>> You may have spotted that the function actually computes the variance rather than +>> the standard deviation. Perhaps that made you think to add the test +>> for some data where the variance and standard deviation are different. +>> In more complex examples, it is often easier to spot code that looks like it could be wrong +>> and think of a test that will exercise it. This saves embarrassment if the code turns out +>> to be right, means you have the missing test written if it is wrong, and is often quicker +>> than trying to execute the code in your head to find out if it is correct. +>> +> {: .solution} +{: .challenge} -Perform the above actions on the pull request you received, -this time acting as the moderator/code reviewer. +### Responding to Review Comments + +When you receive comments on your code, +there are a few different things that can happen. + +1. You understand and agree with what the reviewer is saying. +With such a review, you should make the change to your code on your branch (or accept the +suggested change by the reviewer). +Once you've made the change you can commit it. +It might be helpful to add a thumbs up reaction to the comment, so the reviewer knows +you have addressed it. Even better, leave a comment such as "Fixed via #commit_number" with a link +to your commit that implemented the change. +![Responding to a review comment with an emoji](../fig/github-respond-to-review-comment-with-emoji.png){: .image-with-shadow width="800px"} +![Responding to a review comment with a link to commit](../fig/github-respond-to-review-comment-with-commit-link.png){: .image-with-shadow width="800px"} +2. Some comment may not make total sense to you or it is not clear +what the change should be - you should reply to such comments to ask for clarification. +3. You disagree or are really lost on what the reviewer is driving at with their comments. +In this case, it might be best to talk to them in person. +Discussions that happen on code reviews can often feel quite adversarial - +discussing what the best solution is in person can often defuse this. + +> ## Exercise: Responding to Comments +> +> Look at the pull request that you created on your repository. +> By now you should now have someone else's comments on it. +> For each comment, either reply explaining why you don't think the change is necessary +> or make the change and push a commit fixing it. You can reply to the comment indicating you +> have done it. +> +> At the same time, people will be addressing your comments on a pull request in their repository. +> If you're happy that your comment has been suitably addressed, you can mark it as resolved. +> Once you're happy they have all been addressed, you can approve the pull request. +> To approve a pull request, submit a new review and this time select `Approve`. +> This tells the author you are happy for them to merge the pull request. +{: .challenge} -#### Step 6: Closing a Pull Request +### Approving a Pull Request -1. Once the moderator approves your changes, either one of you can merge onto the base branch. - Typically, it is the responsibility of the code's author to do the merge +1. Once the reviewer approves the changes, the person whose repository it is can + merge the changes onto the base branch. + Typically, it is the code author's responsibility to merge but this may differ from team to team. - ![Merging a pull request in GitHub](../fig/github-merge-pull-request.png){: .image-with-shadow width="900px"} + In our case, you will merge the changes on the PR on your repository. + ![Merging a pull request in GitHub](../fig/github-merge-pull-request.png){: .image-with-shadow width="800px"} 2. Delete the merged branch to reduce the clutter in the repository. -Repeat the above actions for the pull request you received. +## Writing Easy-To-Review Code + +There are a few things you can do to make it +as easy as possible for the reviewer to review your code: + +* Keep the changes **small**. +* Keep each commit as **one logical change**. +* Provide a **clear description** of the change. +* **Review your code yourself**, before requesting a review. + +The most important thing to keep in mind is how long your pull request is. +Smaller changes, that just make one small improvement, will be much quicker and easier to +review. +There is no golden rule, but [studies into code review](https://smartbear.com/resources/ebooks/the-state-of-code-review-2020-report/) show that you should not review more +than 400 lines of code at a time, so this is a reasonable target to aim for. +You can refer to some [studies](https://jserd.springeropen.com/articles/10.1186/s40411-018-0058-0) +and [Google recommendations](https://google.github.io/eng-practices/review/developer/small-cls.html) +as to what a "large pull request" is but be aware that it is not an exact science. + +Even within a single review, try to keep each commit to be making one logical change. +This can help if your review would otherwise be too large. +In particular, if you've reformatted, refactored and changed the behavior of the +code make sure each of these is in a separate commit +(i.e reformat the code, commit, refactor the code, commit, alter the behavior of the code, commit). + +Make sure you write a clear description of the content and purpose of the change. +This should be provided as the pull request description. +This should provide the context needed to read the code. + +It is also a good idea to review your code yourself, +before requesting a review. +In doing this you will spot the more obvious issues with your code, +allowing your reviewer to focus on the things you cannot spot. + +## Writing Effective Review Comments + +Code is written by humans (mostly!), and code review is a form of communication. +As such, empathy is important for effective reviewing. + +When reviewing code, it can be sometimes frustrating when code is confusing, particularly +as it will be implemented differently to how you would have done it. +However, it is important as a reviewer to be compassionate to the +person whose code you are reviewing. +Specifically: + +* Identify positives in code as and when you find them (particularly if it is an improvement on + something you've previously fed back on in a previous review). +* Remember different doesn't mean better - only request changes if the code is wrong or hard to understand. +* Only provide a few non-critical suggestions - you are aiming for better rather than perfect. +* Ask questions to understand why something has been done a certain way rather than assuming you + know a better way. +* If a conversation is taking place on a review and hasn't been resolved by a + single back-and-forth exchange, then schedule a conversation to discuss instead + (recording the results of the discussion in the PR). + +## Defining a Review Process For Your Team + +To be effective, code review needs to be a process that is followed by everyone +developing the code. +Everyone should believe that the process provides value. +One way to foster this is to agree on the review process as a team and consider, e.g.: + +* Do all changes need to go through code review +* What technologies will you use to manage the review process +* How quickly do you expect someone to review the code once you've raised a PR? +* How long should be spent reviewing code? +* What kind of issues are (and aren't) appropriate to raise in a PR? +* How will someone know when they are expected to take action (e.g. review a PR)? + +You could also consider using pull request states in GitHub: + - Open a pull request in a `DRAFT` state to show progress or request early feedback, + - `READY FOR REVIEW` when you are ready for feedback, + - `CHANGES REQUESTED` to let the author know + they need to fix the requested changes or discuss more, + - `APPROVED` to let the author they can merge their pull request. + +Once you've introduced a review process, you should monitor (either formally or +informally) how well it is working. + +It is important that reviews are processed quickly, to avoid costly context switching. +We recommend aiming for 3 hours to get a first review, with the PR being merged the same +day in most cases. If you are regularly missing these targets, then you should review +where things are getting stuck and work out what you can do to move things along. -If the work on the feature branch is completed and it is sufficiently tested, -the feature branch can now be merged into the `develop` branch. +> ## Exercise: Code Review in Your Own Working Environment +> +> In this episode we have looked at why and how to use a tool driven code review process +> using GitHub pull requests. We've also looked at some best practices for doing +> code reviews in general. +> +> Now think about how you typically develop code. +> What benefits do you think you would see for introducing a code review process in +> your work environment. +> How you might institute code review practices within your environment. +> Write down a process for a tool assisted code review, answering the questions +> above. +> +> Once complete, discuss with the rest of the class what are the advantages of +> a code review process and +> what challenges you think you'd face in implementing this process in your own working environment. +> > ## Solution +> > The purposes of code review include: +> > - improving internal code readability, understandability, quality and maintainability, +> > - checking for coding standards compliance, code uniformity and consistency, +> > - checking for test coverage and detecting bugs and code defects early, +> > - detecting performance problems and identifying code optimisation points, +> > - finding alternative/better solutions, +> > - sharing knowledge of the code, and of coding standards and expectations of quality. +> > +> > Finally, it helps increase the sense of collective code ownership and responsibility, +> > which in turn helps increase the "bus factor" +> > and reduce the risk resulting from information and capabilities +> > being held by a single person "responsible" for a certain part of the codebase +> > and not being shared among team members. +> > +> > Challenges you might face introducing a code review process: +> > - complaints that it is a waste of time, +> > - creating a negative atmosphere where people are overly critical of each others work, +> > or are defensive of their own, +> > - perfectionism leading to slower development, +> > - people not sharing code to avoid the review process. +> > +> > Make sure to monitor whether these are happening, and adjust the process accordingly. +> {: .solution} +{: .challenge} -## Best Practice for Code Review +## Further Reading There are multiple perspectives to a code review process - from general practices to technical details relating to different roles involved in the process. -It is critical for the code's quality, stability and maintainability -that the team decides on this process and sticks to it. -Here are some examples of best practices for you to consider -(also check these useful code review blogs from [ +We have discussed the main points, but do check these useful code review blogs from [ Swarmia](https://www.swarmia.com/blog/a-complete-guide-to-code-reviews/?utm_term=code%20review&utm_campaign=Code+review+best+practices&utm_source=adwords&utm_medium=ppc&hsa_acc=6644081770&hsa_cam=14940336179&hsa_grp=131344939434&hsa_ad=552679672005&hsa_src=g&hsa_tgt=kwd-17740433&hsa_kw=code%20review&hsa_mt=b&hsa_net=adwords&hsa_ver=3&gclid=Cj0KCQiAw9qOBhC-ARIsAG-rdn7_nhMMyE7aeSzosRRqZ52vafBOyMrpL4Ypru0PHWK4Rl8QLIhkeA0aAsxqEALw_wcB) -and [Smartbear](https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/)): - -1. Decide the focus of your code review process, e.g., consider some of the following: - - code design and functionality - - does the code fit in the overall design and does it do what was intended? - - code understandability and complexity - - is the code readable and would another developer be able to understand it? - - tests - does the code have automated tests? - - naming - are names used for variables and functions descriptive, - do they follow naming conventions? - - comments and documentation - - are there clear and useful comments that explain complex designs well - and focus on the "why/because" rather than the "what/how"? -2. Do not review code too quickly and do not review for too long in one sitting. - According to - [“Best Kept Secrets of Peer Code Review” (Cohen, 2006)](https://www.amazon.co.uk/Best-Kept-Secrets-Peer-Review/dp/1599160676) - - the first hour of review matters the most as - detection of defects significantly drops after this period. - [Studies into code review](https://smartbear.com/resources/ebooks/the-state-of-code-review-2020-report/) - also show that you should not review more than 400 lines of code at a time. - Conducting more frequent shorter reviews seems to be more effective. -3. Decide on the level of depth for code reviews - to maintain the balance between the creation time and time spent reviewing code - - e.g. reserve them for critical portions of code and avoid nit-picking on small details. - Try using automated checks and linters when possible, - e.g. for consistent usage of certain terminology across the code and code styles. -4. Communicate clearly and effectively - - when reviewing code, be explicit about the action you request from the author. -5. Foster a positive feedback culture: - - give feedback about the code, not about the author - - accept that there are multiple correct solutions to a problem - - sandwich criticism with positive comments and praise -7. Utilise multiple code review techniques - - use email, - pair programming, - over-the-shoulder, - team discussions and - tool-assisted or - any combination that works for your team. - However, for the most effective and efficient code reviews, - tool-assisted process is recommended. -9. From a more technical perspective: - - use a feature branch for pull requests as you can push follow-up commits - if you need to update your proposed changes - - avoid large pull requests as they are more difficult to review. - You can refer to some [studies](https://jserd.springeropen.com/articles/10.1186/s40411-018-0058-0) - and [Google recommendations](https://google.github.io/eng-practices/review/developer/small-cls.html) - as to what a "large pull request" is but be aware that it is not exact science. - - don't force push to a pull request as it changes the repository history - and can corrupt your pull request for other collaborators - - use pull request states in GitHub effectively (based on your team's code review process) - - e.g. in GitHub you can open a pull request in a `DRAFT` state - to show progress or request early feedback; - `READY FOR REVIEW` when you are ready for feedback; - `CHANGES REQUESTED` to let the author know - they need to fix the requested changes or discuss more; - `APPROVED` to let the author they can merge their pull request. +and [Smartbear](https://smartbear.com/learn/code-review/best-practices-for-peer-code-review/). -> ## Exercise: Code Review in Your Own Working Environment -> -> At the start of this episode we briefly looked at a number of techniques for doing code review, -> and as an example, -> went on to see how we can use GitHub Pull Requests to review team member code changes. -> Finally, we also looked at some best practices for doing code reviews in general. -> -> Now think about how you typically develop code, -> and how you might institute code review practices within your own working environment. -> Write down briefly for your own reference (perhaps using bullet points) -> some answers to the following questions: -> -> - Which 2 or 3 key circumstances would code review be most useful for you and your colleagues? -> - Referring to the first section of this episode above, -> which type of code review would be most useful for each circumstance -> (and would work best within your own working environment)? -> - Taking one of these circumstances where code review would be most beneficial, -> how would you organise such a code review, e.g.: -> - Which aspects of the codebase would be the most useful to cover? -> - How often would you do them? -> - How long would the activity take? -> - Who would ideally be involved? -> - Any particular practices you would use? -{: .challenge} +The key thing is to try it, and iterate the process until it works well for your team. {% include links.md %} diff --git a/_layouts/episode_mermaid.html b/_layouts/episode_mermaid.html new file mode 100644 index 000000000..b5ffe5179 --- /dev/null +++ b/_layouts/episode_mermaid.html @@ -0,0 +1,22 @@ +--- +layout: episode +--- +{{ content }} + + + + diff --git a/fig/code-review-sequence-diagram.svg b/fig/code-review-sequence-diagram.svg new file mode 100644 index 000000000..dd57d1ec8 --- /dev/null +++ b/fig/code-review-sequence-diagram.svg @@ -0,0 +1 @@ +ReviewerAuthorReviewerAuthorloop[Until approved]Write some codeRaise a pull requestAdd comments to a reviewSubmit a reviewAddress or respond to review commentsClarify or resolve commentsApprove pull requestMerge pull request \ No newline at end of file diff --git a/fig/copy-template-repository.png b/fig/copy-template-repository.png deleted file mode 100644 index 87b488ab3..000000000 Binary files a/fig/copy-template-repository.png and /dev/null differ diff --git a/fig/exercise-feature-branch.svg b/fig/exercise-feature-branch.svg deleted file mode 100644 index 569ecadd3..000000000 --- a/fig/exercise-feature-branch.svg +++ /dev/null @@ -1,433 +0,0 @@ - - - - - - image/svg+xml - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/fig/github-add-collaborator.png b/fig/github-add-collaborator.png new file mode 100644 index 000000000..6b59686ea Binary files /dev/null and b/fig/github-add-collaborator.png differ diff --git a/fig/github-add-emoji.png b/fig/github-add-emoji.png new file mode 100644 index 000000000..34a5062e6 Binary files /dev/null and b/fig/github-add-emoji.png differ diff --git a/fig/github-create-pull-request.png b/fig/github-create-pull-request.png index 02cf249c7..8cbf5c7c0 100644 Binary files a/fig/github-create-pull-request.png and b/fig/github-create-pull-request.png differ diff --git a/fig/github-finish-pull-request-review.png b/fig/github-finish-pull-request-review.png new file mode 100644 index 000000000..31e030022 Binary files /dev/null and b/fig/github-finish-pull-request-review.png differ diff --git a/fig/github-fork-repository-confirm.png b/fig/github-fork-repository-confirm.png new file mode 100644 index 000000000..12bb399d6 Binary files /dev/null and b/fig/github-fork-repository-confirm.png differ diff --git a/fig/github-fork-repository.png b/fig/github-fork-repository.png new file mode 100644 index 000000000..3eb6cb4e6 Binary files /dev/null and b/fig/github-fork-repository.png differ diff --git a/fig/github-forked-repository-own.png b/fig/github-forked-repository-own.png new file mode 100644 index 000000000..1a4755c2f Binary files /dev/null and b/fig/github-forked-repository-own.png differ diff --git a/fig/github-merge-pull-request.png b/fig/github-merge-pull-request.png index a31a85a96..e69bdf4fc 100644 Binary files a/fig/github-merge-pull-request.png and b/fig/github-merge-pull-request.png differ diff --git a/fig/github-pull-request-add-comment.png b/fig/github-pull-request-add-comment.png new file mode 100644 index 000000000..56d127034 Binary files /dev/null and b/fig/github-pull-request-add-comment.png differ diff --git a/fig/github-pull-request-add-suggestion.png b/fig/github-pull-request-add-suggestion.png new file mode 100644 index 000000000..61326106d Binary files /dev/null and b/fig/github-pull-request-add-suggestion.png differ diff --git a/fig/github-pull-request-files-changed.png b/fig/github-pull-request-files-changed.png new file mode 100644 index 000000000..9a7393985 Binary files /dev/null and b/fig/github-pull-request-files-changed.png differ diff --git a/fig/github-pull-request-tab.png b/fig/github-pull-request-tab.png new file mode 100644 index 000000000..9a5c7751d Binary files /dev/null and b/fig/github-pull-request-tab.png differ diff --git a/fig/github-respond-to-review-comment-with-commit-link.png b/fig/github-respond-to-review-comment-with-commit-link.png new file mode 100644 index 000000000..5ef00d4d2 Binary files /dev/null and b/fig/github-respond-to-review-comment-with-commit-link.png differ diff --git a/fig/github-respond-to-review-comment-with-emoji.png b/fig/github-respond-to-review-comment-with-emoji.png new file mode 100644 index 000000000..7d6124ad4 Binary files /dev/null and b/fig/github-respond-to-review-comment-with-emoji.png differ diff --git a/fig/github-submit-pull-request-review.png b/fig/github-submit-pull-request-review.png new file mode 100644 index 000000000..65321aef2 Binary files /dev/null and b/fig/github-submit-pull-request-review.png differ diff --git a/fig/github-submit-pull-request.png b/fig/github-submit-pull-request.png new file mode 100644 index 000000000..59f7cce2e Binary files /dev/null and b/fig/github-submit-pull-request.png differ diff --git a/fig/own-template-repository.png b/fig/own-template-repository.png deleted file mode 100644 index 0959357ce..000000000 Binary files a/fig/own-template-repository.png and /dev/null differ diff --git a/fig/template-repository.png b/fig/template-repository.png deleted file mode 100644 index 1f6554d2f..000000000 Binary files a/fig/template-repository.png and /dev/null differ