Guidance for Pull Requests into the COMMON Repository

Last updated on December 11, 2020.

Last Reviewed and Approved on PENDING REVIEW

Introduction

Pull Requests (PR) allow a developer the opportunity to have multiple sets of eyes on their code in a collaborative space. This helps to ensure the best possible code is deployed into the live environment. These instructions provide guidance on how to submit a Pull Request in the RPS Common Repository (Repo).

Assumptions

  1. You have access to the Mission Network RPS Azure DevOps instance at www.devops.peoc3t.com.
  2. You are submitting a PR to the COMMON repo NOT the Core Repo.
  3. You are using and familiar with Visual Studio to create/edit/submit code changes. Pull Request Etiquette

Pull Request Etiquette

Sizing

• Simplify the amount of changes in your pull request. A large pull request (>100 lines of code changed) usually indicates that the user story was not properly decomposed. o Smaller PRs allow for quicker reviews and subsequent merges to the default branch, leading to increased work velocity o Smaller PRs allow for quicker defect identification and mitigation, since the scope of changes are much more discrete

Naming

• Provide meaningful descriptions for every commit. This assists PR reviewers to better understand the context of code changes and for version history o For some great guidance, see: https://chris.beams.io/posts/git-commit/

Expectations

• Be polite. Conversations should occur as they would if you were in person. • This is a team environment and success is dependent upon being accountable to each other. If you are assigned a PR, do your best to perform the review in a timely manner. If a blocker exists, let your scrum master, project manager or PR creator know as soon as possible. • When making a comment as a PR reviewer, take the time to make a recommendation. This will help others grow and potentially avoid the same mistake in the future.

Culture

• Code reviews are a safe place and meant to be a tool for communication. • Review the code, not the person. As a reviewer, be aware that your sole job is to review the changes, not make assumptions about someone’s personality. • On the flip side, as the owner of the code review understand that any feedback you are getting should not be considered a personal attack on you. It is merely a review of the code. • Leave the communication channel open for discussion as long as needed. Marking comments resolved without a chance for the commenter or owner to reply closes that channel prematurely when it could’ve otherwise provided more valuable conversation.

PR Process

Figure 1: Common Repo PR Process

Note: It is critical to ensure you are working in the Common repo in ADO and the Common folder in Visual Studio. Figure 2: Common Repo Selection

Branch Creation

Before you begin development, a branch should be created for the work in Visual Studio.

Branch Naming

All branch names for Backlog Item-level work should:

  1. Start with "feature/"
  2. Followed by the TFS alias of the developer submitting the PR and a hyphen
  3. Followed by the backlog item number surrounded by hyphens (see example below)
  4. Followed by a short (2-3 word) description of the work
  5. Output: feature/jskerrett-23456-WriteDocumentation

Creating the Branch

Take the following steps in Visual Studio:

  1. Open Team Explorer
  2. From the Team Explorer Home page, select Branches Figure 3: Common Repo Branch Naming1
  3. Click on New Branch Figure 4: Common Repo New Branch
  4. Type the name of the PR
  5. Click on Create Branch Figure 5: Common Repo New PR1

Pull Request Creation

Once your development changes are finished, a pull request should be created to begin the process of merging the proposed changes to the default (‘develop’) branch. The high level process begins at the PEOC3T ADO site and ends with a pull request generated. Figure 6: Common Repo New PR2

  1. To begin creating a Pull requests in Azure DevOps, Navigate to https://www.devops.peoc3t.com Figure 7: Common Repo New PR3
  2. Click on the Pull requests Menu item in the left menu bar
  3. Click New pull request button in the top right corner of the screen Figure 8: Common Repo New PR4
  4. Click the Select a source branch… dropdown list
  5. Select the branch to be merged. Figure 9: Common Repo New PR5
  6. Fill in the required fields: a. Descriptive title b. Fill out the Description field c. Ensure a work item is linked to the PR by selecting the appropriate backlog item from the ‘Work Items’ dropdown list d. Click the Create button at the bottom of the New Pull Request Form Figure 10: Common Repo PR Review Form
  7. Pull Request Has now been created

PR Approval

Approval Requirements

The following criteria must be met before a PR can be completed and merged to the default branch:

  • PR must be approved by 2 required approvers
    • For COMMON repository, the required approvers are limited to the MCS ‘Required Approvers’ group
    • Other approvals on the PR are recommended, as more eyes on code changes not only increase confidence in the change, but also increase awareness of changes to the codebase
  • PR cannot be approved with a blocking approver
  • Reviewers cannot approve their own changes
  • A work item (backlog item or bug) must be linked to the PR
    • This creates historical context for code changes
  • All comments must be resolved
  • CI Builds must all pass
    • PowerShell Unit tests
    • PowerShell Meta tests
    • DSC Modules tests

Figure 11: Common Repo PR Review Policy

Currently, all GD users will have access to view and approve PRs into the COMMON repository.

Note: If the PR has passed the Policy requirement, a green check mark will show beside it. If the PR has not passed the policy requirement, then a red X will show beside it.

Figure 12: Common Repo PR Review Policy Approval

Completing the PR - Definition of Done

For a Work Item (at the Product Backlog Item or Bug level) to be considered done, the following must be addressed as part of the Pull Request:

  • The latest updates from ‘develop’ are merged into the working "feature" branch prior to PR creation
  • Altered code meets Style Guidelines
  • Unit and/or Integration Test(s) implemented and/or updated
  • All tests are run locally and pass
  • For non-trivial updates, a lab is deployed locally to ensure there are no breaking changes. RPS functionality is verified.
  • All Work Item Acceptance Criteria met or discussed in work item (and tagged Dev Lead/Product Owner)
  • (Runbooks) Test Runbook within TMS from either TaskMap or Schedule
  • (Runbooks) Has standardized input (TaskAssignmentId), Target loading
  • DSC Resource updates are documented and communicated
  • Applicable documentation updated/created
  • OSS Registrations created for any new Open Source software/version
    • If new / updated 3rd Party / OSS, update the file at $/Documents/ThirdPartyNotices.txt
  • Update Release Notes, as appropriate, with each PR
  • Product Backlog Item/Bug moved to Done state when PR is Approved and Completed