class: middle background-image: url(/assets/devcop-slides/background-title.jpg) .title[ # Dev CoP Presentation: Pull Requests ] .right[] --- ## Intro In previous DevCoP sessions we talked about moving to Git, and the point of that is to leverage branching strategies (more light weight than a centralized repository TFVC). The November DevCoP presentation was about branching strategies. This presentation covers how to manage the merging of these branches in your workflow thru a process called “Pull Requests”. NOTE: "Pull Request" are also known as "Merge Request" --- ## What is a Pull Request ? -- A “Pull Request” allows you to visualize and collaborate on the proposed changes to source code that exist as commits on a given branch. They are a request to merge one branch into another. They help a team establish, enforce and verify policies on all code changes. Example: <https://gccode.ssc-spc.gc.ca/iitb-dgiit/sds/GOCWebTemplates/DotNetTemplates/merge_requests/230> --- Example   --- ## CI-CD Pipeline The Pull Request process covers the: Execute Tests, Code Review, Approve Changes & Merge steps within the Senior Advisors recommended CI-CD pipeline flow. [cicd](cicd.md)  --- ## What is the goal of a Pull Request ? .answer[ - Manage the merging of branches in your workflow. - Changes get verified thru automation and your colleagues. - A big part of the Pull Request is the code review. ] --- ## What is the goal of a Code Review? The primary goal of a code review should be collaboration. The collaboration allows the reviewer and author of the code change to learn more about the project, enabling them both to make better changes to the project in the moment and in the future. .answer[ - Does the code make sense? - Were multiple features combined? - Was debugging code removed? ] Note: Remember to stay courteous, the code is being reviewed not the person. --- ## Roles and responsibilities of Author and reviewer ### Author -- .answer[ - The first reviewer of your code is you. - Keep Code review short: Contains a single task. Don't mix features. [merging-review.html](/guides/merging-review.html) - Describe the changes in the Pull Request. Link to a task. - New functions and conditions come with new unit tests. - Changed functions and conditions don't affect existing unit tests. - Complex sections of code contains comments. ] --- ### Reviewer -- .answer[ - Review the code as soon as possible. - When approving a merge, you are just as responsible for the changes as the Author. - Ask for clarifications if changes are ambiguous. - Be effective in comments, Recommend changes in a constructive manner. - Be aware of potential design flaws - Be aware of alternate existing functionalities that could be suggested. ] --- ## How do I keep reviews from taking up all my time? .answer[ - Keep the changes small: If the change is too large and could have been broken apart, reject it. - Don’t validate code : Let the pipeline do that work for you. Focus on learning what it is the author is trying to do, and check that what they did makes sense. - Ask for more details : Don’t spend time trying to figure it out, it should be simple enough (or explained in comments) to understand in a minute or two. ]  --- ### Valid "big" pull request In some cases, “big pull requests” are inevitable but can still be time effective if the scope is limited to one goal. Here is an example of a manageable “big pull request”, it’s about refactoring the code.  --- ## Types of Merges Once the Reviewer has approved the code changes. Anyone with the appropriate rights can merge the changes into the parent branch. Note: Depending on the platform there might be variations.  --- ### Merge Commit Merge commits: Retains all of the commits in your branch and interleaves them with commits on the base branch.  --- ### Squash and Merge Squash and Merge: Retains the changes but omits the individual commits from history.  --- ### Rebase and Merge Rebase and Merge: This moves the entire feature branch to begin on the tip of the master branch, effectively incorporating all of the new commits in master  If you want more in depth knowledge about merge types please let us know. You may get more details in the following link: [merging-review.html](/guides/merging-review.html) --- ## Conclusion The use of Pull requests increases: Quality, Collaboration and Knowledge --- class: middle, center background-image: url(/assets/devcop-slides/background-image.jpg) ## We are here to help! .okay[If you want a more in-depth disscution on this, please contact us!] --- class: middle, center background-image: url(/assets/devcop-slides/background-image.jpg) --- class: middle, center background-image: url(/assets/devcop-slides/background-image.jpg) ## Questions