{"id":11616,"date":"2017-07-20T13:37:26","date_gmt":"2017-07-20T13:37:26","guid":{"rendered":"https:\/\/www.process.st\/templates\/github-pull-request-procedure\/"},"modified":"2024-02-28T20:45:54","modified_gmt":"2024-02-28T20:45:54","slug":"github-pull-request-procedure","status":"publish","type":"post","link":"https:\/\/www.process.st\/templates\/github-pull-request-procedure\/","title":{"rendered":"GitHub Pull Request Procedure"},"content":{"rendered":"<section id=\"introduction\">\n<h2> Introduction: <\/h2>\n<div class=\"text-content\">\n<p>To avoid errors and ensure you're keeping your GitHub records useful and readable, you need to know the correct way to make a pull request. This checklist will make sure the issue is properly tested (for both functionality and user experience), and also guide you through the review process.<\/p>\n<p>There are some optional steps in this checklist that are designed to help passively squash low-priority bugs. Feel free to remove step 8 if you're not concerned with that, or if you want to make the process go faster. Since the PR process is run so often, it's a great place to insert small steps like this and whittle down your bug backlog.<\/p>\n<\/p><\/div>\n<\/section>\n<section id=\"make-sure-you-finished-the-issue\">\n<h2> Make sure you finished the issue <\/h2>\n<div class=\"text-content\">\n<p>Re-read the summary and description of the issue in JIRA and make sure that you finished it.<\/p>\n<\/p><\/div>\n<\/section>\n<section id=\"make-sure-to-thoroughly-test-the-issue\">\n<h2> Make sure to thoroughly test the issue <\/h2>\n<div class=\"text-content\">\n<p><u>Please do not take this step lightly.<\/u><\/p>\n<p>This is very important. The majority of issues that fail review are because something was&nbsp;not tested carefully.<\/p>\n<p>Make sure that the thing you have fixed is indeed fixed.<\/p>\n<p>For example, if your issue was&nbsp;to track a field in Intercom, then make sure that the field is actually being sent to Intercom and shows up.<\/p>\n<\/p><\/div>\n<\/section>\n<section id=\"check-usages-of-modified-methods\">\n<h2> Check usages of modified methods <\/h2>\n<div class=\"text-content\">\n<p>It's possible that during code refactoring some methods were changed, we need to check their usages and make sure that logic is not broken.<\/p>\n<\/p><\/div>\n<div class=\"multi-select-content form-field-content\">\n<ul class=\"items\">\n<li class=\"item\">\n<div class=\"step-number-container\">\n<div class=\"step-number\">\n       1\n     <\/div>\n<\/p><\/div>\n<div class=\"step-checkbox-container\">\n<div class=\"step-checkbox\"><\/div>\n<\/p><\/div>\n<div class=\"item-name-static\">\n      Check usages of modified Service methods\n    <\/div>\n<\/li>\n<li class=\"item\">\n<div class=\"step-number-container\">\n<div class=\"step-number\">\n       2\n     <\/div>\n<\/p><\/div>\n<div class=\"step-checkbox-container\">\n<div class=\"step-checkbox\"><\/div>\n<\/p><\/div>\n<div class=\"item-name-static\">\n      Check usages of modified Repository methods\n    <\/div>\n<\/li>\n<\/ul><\/div>\n<\/section>\n<section id=\"dont-forget-to-test-on-mobile-for-anything-visual-that-you-change\">\n<h2> Don't forget to test on mobile for anything visual that you change <\/h2>\n<div class=\"text-content\">\n<p>Chrome developer console has a button where you can resize the screen to the mobile size. This is a way to see if anything is looking weird.<\/p>\n<p>If it's a visual change, make sure it's working on all browsers too. You can check with&nbsp;<a href=\"https:\/\/caniuse.com\/\" rel=\"nofollow noopener\" target=\"_blank\">https:\/\/caniuse.com\/<\/a>&nbsp;or <a href=\"https:\/\/developer.mozilla.org\/en-US\/docs\/Web\/CSS\/Reference\" rel=\"nofollow noopener\" target=\"_blank\">Mozilla reference guide<\/a>.&nbsp;<\/p>\n<p><strong>For visual changes:<\/strong><\/p>\n<\/p><\/div>\n<div class=\"multi-select-content form-field-content\">\n<ul class=\"items\">\n<li class=\"item\">\n<div class=\"step-number-container\">\n<div class=\"step-number\">\n       1\n     <\/div>\n<\/p><\/div>\n<div class=\"step-checkbox-container\">\n<div class=\"step-checkbox\"><\/div>\n<\/p><\/div>\n<div class=\"item-name-static\">\n      Check Chrome\n    <\/div>\n<\/li>\n<li class=\"item\">\n<div class=\"step-number-container\">\n<div class=\"step-number\">\n       2\n     <\/div>\n<\/p><\/div>\n<div class=\"step-checkbox-container\">\n<div class=\"step-checkbox\"><\/div>\n<\/p><\/div>\n<div class=\"item-name-static\">\n      Check Internet Explorer\n    <\/div>\n<\/li>\n<li class=\"item\">\n<div class=\"step-number-container\">\n<div class=\"step-number\">\n       3\n     <\/div>\n<\/p><\/div>\n<div class=\"step-checkbox-container\">\n<div class=\"step-checkbox\"><\/div>\n<\/p><\/div>\n<div class=\"item-name-static\">\n      Check Safari\n    <\/div>\n<\/li>\n<li class=\"item\">\n<div class=\"step-number-container\">\n<div class=\"step-number\">\n       4\n     <\/div>\n<\/p><\/div>\n<div class=\"step-checkbox-container\">\n<div class=\"step-checkbox\"><\/div>\n<\/p><\/div>\n<div class=\"item-name-static\">\n      Check Firefox\n    <\/div>\n<\/li>\n<\/ul><\/div>\n<\/section>\n<section id=\"run-all-tests-to-make-sure-theyre-passing\">\n<h2> Run all tests to make sure they're passing <\/h2>\n<div class=\"text-content\">\n<p>For the front-end,&nbsp;run grunt-test:<\/p>\n<pre>grunt test<\/pre>\n<\/p><\/div>\n<\/section>\n<section id=\"make-sure-the-pr-contains-a-new-test\">\n<h2> Make sure the PR contains a new test <\/h2>\n<div class=\"text-content\">\n<p>The test does not have to be big.<\/p>\n<\/p><\/div>\n<\/section>\n<section id=\"if-be-make-sure-the-pr-removes-1-scalastyle-issue\">\n<h2> If BE: Make sure the PR removes 1 Scalastyle issue <\/h2>\n<div class=\"text-content\">\n<p>We're trying to get rid of all Scalastyle issues, so make sure you remove one issue when making a BE PR.<\/p>\n<p>To see a list of all issues, type&nbsp;<strong>sbt scalastyle<\/strong>.<\/p>\n<\/p><\/div>\n<\/section>\n<section id=\"run-code-formatters-if-applicable\">\n<h2> Run code formatters if applicable <\/h2>\n<div class=\"text-content\">\n<p>If it's&nbsp;a BE PR, then run this:<\/p>\n<pre>sbt fmt<\/pre>\n<p>If it&nbsp;fails, then so will CirlceCI. If it doesn't seem to be formatting correctly you can&nbsp;go in to the offending file and use Command-Shift-L (or Ctrl-Shift-L if not on Mac) or do&nbsp;<strong>sbt clean<\/strong> and then start over from the top.<\/p>\n<p>If it's a FE PR, then run this:&nbsp;<\/p>\n<pre>grunt lint<\/pre>\n<p>Fix any errors (if you don't, CircleCI will squawk).This will run a javascript linter, and also a SCSS linter.<\/p>\n<p><strong>SCSS Guidelines:<\/strong><\/p>\n<ul>\n<li><strong><\/strong>Make sure&nbsp;there is a namespace for each new&nbsp;scss&nbsp;file:\n<pre>ex:<br \/>.feature {<br \/>  .feature-component {<br \/><br \/>  }<br \/>}<\/pre>\n<\/li>\n<li>Make sure the class is saying what the element does vs. using another class because you already have it (Make it semantic).<\/li>\n<li>CSS properties should be added in alphabetical order.<\/li>\n<li>Avoid vendor prefixes, use bourbon mix-ins when needed instead. @includes go above the alphabetical ordering.<\/li>\n<li>Colors should be written as lowercase HEX values. Ex: #fff vs. white or #FFFFFF<\/li>\n<li>Avoid nesting selectors too deeply.<\/li>\n<li>\n<p>Use `\/\/` comments everywhere<\/p>\n<\/li>\n<li>\n<p>Leading Zeros: `0.8` should be written without a leading zero as `.8`<\/p>\n<\/li>\n<li>Avoid qualifying elements in selectors (also known as &quot;tag-qualifying&quot;).<\/li>\n<li>Prefer the shortest shorthand form possible for properties that support it.\n<ul>\n<li>Ex: margin: 1px; vs margin: 1px 1px 1px 1px;<\/li>\n<\/ul>\n<\/li>\n<\/ul>\n<p>You can read the entire lint list here:<\/p>\n<p><a href=\"https:\/\/github.com\/brigade\/scss-lint\/tree\/master\/lib\/scss_lint\/linter\" rel=\"nofollow noopener\" target=\"_blank\">https:\/\/github.com\/brigade\/scss-lint\/tree\/master\/lib\/scss_lint\/linter<\/a><\/p>\n<\/p><\/div>\n<\/section>\n<section id=\"create-a-pr-in-github\">\n<h2> Create a PR in GitHub <\/h2>\n<div class=\"text-content\">\n<p>Here is a link to generate a BE PR (doesn't work for forks):<br \/><a href=\"https:\/\/github.com\/process-street\/process-street\/compare\/develop...{{checklist.name}}\" rel=\"nofollow noopener\" target=\"_blank\">https:\/\/github.com\/[account]\/[repo]\/compare\/develop...{{checklist.name}}<\/a><\/p>\n<\/p><\/div>\n<div class=\"text-content\">\n<p>Here is a link to generate a FE PR (doesn't work for forks):<br \/><a href=\"https:\/\/github.com\/process-street\/front-street\/compare\/develop...{{checklist.name}}\" rel=\"nofollow noopener\" target=\"_blank\">https:\/\/github.com\/[account]\/[repo]\/compare\/develop...{{checklist.name}}<\/a><\/p>\n<p><em>[Edit the link above to include account\/repo in place of placeholders]<\/em><\/p>\n<p>Here are specific links for each developers fork:<\/p>\n<p><em>[Add links to your developer's forks here]<\/em><\/p>\n<\/p><\/div>\n<div class=\"multi-select-content form-field-content\">\n<ul class=\"items\">\n<li class=\"item\">\n<div class=\"step-number-container\">\n<div class=\"step-number\">\n       1\n     <\/div>\n<\/p><\/div>\n<div class=\"step-checkbox-container\">\n<div class=\"step-checkbox\"><\/div>\n<\/p><\/div>\n<div class=\"item-name-static\">\n      Make sure PR is against the develop branch (if it exists) otherwise the staging branch\n    <\/div>\n<\/li>\n<li class=\"item\">\n<div class=\"step-number-container\">\n<div class=\"step-number\">\n       2\n     <\/div>\n<\/p><\/div>\n<div class=\"step-checkbox-container\">\n<div class=\"step-checkbox\"><\/div>\n<\/p><\/div>\n<div class=\"item-name-static\">\n      Make sure that the PR is mergable without conflicts\n    <\/div>\n<\/li>\n<li class=\"item\">\n<div class=\"step-number-container\">\n<div class=\"step-number\">\n       3\n     <\/div>\n<\/p><\/div>\n<div class=\"step-checkbox-container\">\n<div class=\"step-checkbox\"><\/div>\n<\/p><\/div>\n<div class=\"item-name-static\">\n      The PR title must start with the case-sensitive issue #, e.g. PS-1337 or PUB-101\n    <\/div>\n<\/li>\n<li class=\"item\">\n<div class=\"step-number-container\">\n<div class=\"step-number\">\n       4\n     <\/div>\n<\/p><\/div>\n<div class=\"step-checkbox-container\">\n<div class=\"step-checkbox\"><\/div>\n<\/p><\/div>\n<div class=\"item-name-static\">\n      The PR title must have a short description like, e.g. &quot;PS-1337 Fixes issue with sidebar display&quot;\n    <\/div>\n<\/li>\n<\/ul><\/div>\n<\/section>\n<section id=\"attach-this-checklist-link-as-a-comment-in-the-jira-issue\">\n<h2> Attach this checklist link as a comment in the JIRA issue <\/h2>\n<\/section>\n<section id=\"move-the-issue-from-in-progress-to-in-review\">\n<h2> Move the issue from In Progress to In Review <\/h2>\n<div class=\"text-content\">\n<p>Make sure to assign it to a reviewer who will understand the changes, and try to assign it to someone who has fewer reviews in their queue.<\/p>\n<\/p><\/div>\n<\/section>\n<section id=\"assign-this-task-to-reviewer\">\n<h2> Assign this task to reviewer <\/h2>\n<div class=\"text-content\">\n<p><strong>Do not check this until the review has happened.<\/strong><\/p>\n<p>Contact your reviewer and coordinate&nbsp;a time to demonstrate your feature or bugfix.<\/p>\n<p>If you have several things to review, try to do them all at once.<\/p>\n<p>Please do not waste each other's time by making sure that you've fixed and tested the issue carefully.<\/p>\n<p>When you are reviewing, asking the developer what they did, why they did it, and how they did it.<\/p>\n<\/p><\/div>\n<\/section>\n<section id=\"give-yourself-a-self-five\">\n<h2> Give yourself a self five <\/h2>\n<div class=\"image-content\">\n<figure>\n   <a href=\"https:\/\/st-process-production.s3.amazonaws.com\/776ae393-88ff-41e4-ab14-818b2860215b\/naXrYfP1nx-X8CaJ8KtAEQ.gif\" alt=\"Self five! You've earned it.\" target=\"_blank\" rel=\"noopener\"> <img decoding=\"async\" loading=\"lazy\" src=\"https:\/\/st-process-production.s3.amazonaws.com\/776ae393-88ff-41e4-ab14-818b2860215b\/naXrYfP1nx-X8CaJ8KtAEQ.gif\" \/> <\/a><figcaption>\n     Self five! You've earned it.<br \/>\n   <\/figcaption><\/figure>\n<\/p><\/div>\n<\/section>\n<section id=\"sources\">\n<h2> Sources: <\/h2>\n<div class=\"text-content\">\n<ul>\n<li><a href=\"https:\/\/help.github.com\" rel=\"nofollow noopener\" target=\"_blank\">GitHub Help<\/a> - <a href=\"https:\/\/help.github.com\/articles\/about-pull-requests\/\" rel=\"nofollow noopener\" target=\"_blank\">About Pull Requests<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\" rel=\"nofollow noopener\" target=\"_blank\">Process Street<\/a> - <a href=\"https:\/\/www.process.st\/agile-processes\/\" rel=\"nofollow noopener\" target=\"_blank\">The 11 Agile Processes We Use to Run an Efficient Software Team<\/a><\/li>\n<\/ul><\/div>\n<\/section>\n<section id=\"related-checklists\">\n<h2> Related checklists: <\/h2>\n<div class=\"text-content\">\n<ul>\n<li><a href=\"https:\/\/www.process.st\/templates\/pull-request-review-failed-procedure\/\" rel=\"nofollow\">Pull Request Review Failed Procedure<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/daily-standup-meeting-checklist\/\" rel=\"nofollow\">Daily Standup Meeting Checklist<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/sprint-turnover-process\" rel=\"nofollow\">Sprint Turnover Process<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/sprint-retrospective-process\/\" rel=\"nofollow\">Sprint Retrospective Process<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/sprint-estimation-process\/\" rel=\"nofollow\">Sprint Estimation Process<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/sprint-planning\/\" rel=\"nofollow\">Sprint Planning Process<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/scrum-project-management\/\" rel=\"nofollow\">Scrum Project Management<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/git-workflow\/\" rel=\"nofollow\">Git Workflow<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/user-story-template\/\" rel=\"nofollow\">User Story Template<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/software-deployment\/\" rel=\"nofollow\">Software Deployment<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/software-testing-tutorial\/\" rel=\"nofollow\">Software Testing Tutorial<\/a><\/li>\n<li><a href=\"https:\/\/www.process.st\/templates\/software-debugging-process\/\" rel=\"nofollow\">Software Debugging Process<\/a><\/li>\n<\/ul><\/div>\n<\/section>\n","protected":false},"excerpt":{"rendered":"<p>Introduction: To avoid errors and ensure you're keeping your GitHub records useful and readable, you need to know the correct way to make a pull request. This checklist will make sure the issue is properly tested (for both functionality and user experience), and also guide you through the review process. There are some optional steps [&hellip;]<\/p>\n","protected":false},"author":3,"featured_media":0,"comment_status":"closed","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"_acf_changed":false,"ep_exclude_from_search":false,"cover_icon_emoji":"","cover_icon_url":"","tasks_count":"16","template_description":"This should be used for all PRs to reduce errors, ensure thorough testing, and start the review process.","template_id":"pAofnU7OeTcUz7Y1N8hOIA","task_0":"Introduction:","task_slug_0":"introduction","task_1":"Make sure you finished the issue","task_slug_1":"make-sure-you-finished-the-issue","task_2":"Make sure to thoroughly test the issue","task_slug_2":"make-sure-to-thoroughly-test-the-issue","task_3":"Check usages of modified methods","task_slug_3":"check-usages-of-modified-methods","task_4":"Don't forget to test on mobile for anything visual that you change","task_slug_4":"dont-forget-to-test-on-mobile-for-anything-visual-that-you-change","task_5":"Run all tests to make sure they're passing","task_slug_5":"run-all-tests-to-make-sure-theyre-passing","task_6":"Make sure the PR contains a new test","task_slug_6":"make-sure-the-pr-contains-a-new-test","task_7":"If BE: Make sure the PR removes 1 Scalastyle issue","task_slug_7":"if-be-make-sure-the-pr-removes-1-scalastyle-issue","task_8":"Run code formatters if applicable","task_slug_8":"run-code-formatters-if-applicable","task_9":"Create a PR in GitHub","task_slug_9":"create-a-pr-in-github","task_10":"Attach this checklist link as a comment in the JIRA issue","task_slug_10":"attach-this-checklist-link-as-a-comment-in-the-jira-issue","task_11":"Move the issue from In Progress to In Review","task_slug_11":"move-the-issue-from-in-progress-to-in-review","task_12":"Assign this task to reviewer","task_slug_12":"assign-this-task-to-reviewer","task_13":"Give yourself a self five","task_slug_13":"give-yourself-a-self-five","task_14":"Sources:","task_slug_14":"sources","task_15":"Related checklists:","task_slug_15":"related-checklists","task_16":"","task_slug_16":"","task_17":"","task_slug_17":"","task_18":"","task_slug_18":"","task_19":"","task_slug_19":"","task_20":"","task_slug_20":"","task_21":"","task_slug_21":"","task_22":"","task_slug_22":"","task_23":"","task_slug_23":"","task_24":"","task_slug_24":"","task_25":"","task_slug_25":"","task_26":"","task_slug_26":"","task_27":"","task_slug_27":"","task_28":"","task_slug_28":"","task_29":"","task_slug_29":"","task_30":"","task_slug_30":"","task_31":"","task_slug_31":"","task_32":"","task_slug_32":"","task_33":"","task_slug_33":"","task_34":"","task_slug_34":"","task_35":"","task_slug_35":"","task_36":"","task_slug_36":"","task_37":"","task_slug_37":"","task_38":"","task_slug_38":"","task_39":"","task_slug_39":"","task_40":"","task_slug_40":"","task_41":"","task_slug_41":"","task_42":"","task_slug_42":"","task_43":"","task_slug_43":"","task_44":"","task_slug_44":"","task_45":"","task_slug_45":"","task_46":"","task_slug_46":"","task_47":"","task_slug_47":"","task_48":"","task_slug_48":"","task_49":"","task_slug_49":"","task_50":"","task_slug_50":"","task_51":"","task_slug_51":"","task_52":"","task_slug_52":"","task_53":"","task_slug_53":"","task_54":"","task_slug_54":"","task_55":"","task_slug_55":"","task_56":"","task_slug_56":"","task_57":"","task_slug_57":"","task_58":"","task_slug_58":"","task_59":"","task_slug_59":"","task_60":"","task_slug_60":"","task_61":"","task_slug_61":"","task_62":"","task_slug_62":"","task_63":"","task_slug_63":"","task_64":"","task_slug_64":"","task_65":"","task_slug_65":"","task_66":"","task_slug_66":"","task_67":"","task_slug_67":"","task_68":"","task_slug_68":"","task_69":"","task_slug_69":"","task_70":"","task_slug_70":"","task_71":"","task_slug_71":"","task_72":"","task_slug_72":"","task_73":"","task_slug_73":"","task_74":"","task_slug_74":"","task_75":"","task_slug_75":"","task_76":"","task_slug_76":"","task_77":"","task_slug_77":"","task_78":"","task_slug_78":"","task_79":"","task_slug_79":"","task_80":"","task_slug_80":"","task_81":"","task_slug_81":"","task_82":"","task_slug_82":"","task_83":"","task_slug_83":"","task_84":"","task_slug_84":"","task_85":"","task_slug_85":"","task_86":"","task_slug_86":"","task_87":"","task_slug_87":"","task_88":"","task_slug_88":"","task_89":"","task_slug_89":"","task_90":"","task_slug_90":"","task_91":"","task_slug_91":"","task_92":"","task_slug_92":"","task_93":"","task_slug_93":"","task_94":"","task_slug_94":"","task_95":"","task_slug_95":"","task_96":"","task_slug_96":"","task_97":"","task_slug_97":"","task_98":"","task_slug_98":"","task_99":"","task_slug_99":"","footnotes":""},"categories":[7,1],"tags":[],"class_list":["post-11616","post","type-post","status-publish","format-standard","hentry","category-miscellaneous","category-uncategorized"],"acf":[],"aioseo_notices":[],"_links":{"self":[{"href":"https:\/\/www.process.st\/templates\/wp-json\/wp\/v2\/posts\/11616","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/www.process.st\/templates\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/www.process.st\/templates\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/www.process.st\/templates\/wp-json\/wp\/v2\/users\/3"}],"replies":[{"embeddable":true,"href":"https:\/\/www.process.st\/templates\/wp-json\/wp\/v2\/comments?post=11616"}],"version-history":[{"count":0,"href":"https:\/\/www.process.st\/templates\/wp-json\/wp\/v2\/posts\/11616\/revisions"}],"wp:attachment":[{"href":"https:\/\/www.process.st\/templates\/wp-json\/wp\/v2\/media?parent=11616"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/www.process.st\/templates\/wp-json\/wp\/v2\/categories?post=11616"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/www.process.st\/templates\/wp-json\/wp\/v2\/tags?post=11616"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}