GitHub PR reviews for VIP Go sites

VIP Go platform specific

This document is for sites running on VIP Go.

Learn more

Overview #

For sites on Enhanced or Full levels of code review, iterative code changes of under 1,000 lines of PHP, JS, and SVG code are reviewed in a queue. This workflow uses GitHub’s pull request (PR) functionality, and the associated approval workflow for PRs that GitHub provides. Larger changesets will need to undergo a scheduled review.

GitHub Flow is a branch-based workflow that streamlines reviews and deployment. The diagram above describes the workflow:

  1. When you want to work on a bug fix or new feature, create and switch to a new branch locally.
  2. Develop, make changes, commit, and as desired push up the branch.
  3. When your branch is ready for VIP review, open a Pull Request against the master branch (for production sites).
  4. Pull requests to master surface as a new review request in our internal review queue. The VIP Team will then review the PR directly on GitHub, and suggest any changes by leaving comments. Your team will go through and make any necessary changes, and when you’re ready for another round of review, “dismiss” the review. This process continues until there are no outstanding issues, at which point the VIP Team will “approve” the request. Communication for these status changes will flow through your GitHub repository.
  5. Once the Pull Request is approved by the VIP team, you may merge it to master at your convenience. Merging to master triggers an automatic deployment to the production site.

If there are any issues with the deployment, GitHub provides an easy, one-click option to “revert” Pull Requests (GitHub documentation on reverting a pull request).

GitHub allows any number of approvals against a Pull Request, so your team can also utilize this approvals workflow if desired. Please note, however, that GitHub will allow you to merge your PR to master once the PR has any approval from either the client or VIP team; please wait for approval from a VIP Reviewer before merging the PR to master.

Summary of the flow #

  1. Create a local branch.
  2. Commit, test, push.
  3. When ready for review, create Pull Request (which initiates a review request).
  4. VIP Team reviews Pull Request.
  5. If changes are required, VIP marks Pull Request as “Request Changes”. When your changes are implemented, you “Dismiss Review” to request another review.
  6. If code is ready, VIP marks Pull Request as “Approved”.
  7. You can now merge the Pull Request to master, which triggers a deploy.

↑ Top ↑

Benefits of using GitHub Flow #

GitHub’s code review tools are excellent. There are many benefits to your team and to VIP in using GitHub directly for code review, including:

  • Inline commenting on code, making reviews easier to follow, and sharing feedback more direct.
  • More control over deployments to production. You can choose to merge when you’re ready for the code to be deployed, allowing you to stage changes and deploy when your team is ready.
  • Simplified rollbacks/reversions with the GitHub one-click method to revert Pull Requests.

↑ Top ↑

What about non-production environments? #

Your non-production site(s) and workflow(s) remains as-is. For example, merges to the develop branch deploy automatically to the develop site without VIP involvement.

We highly recommend following GitHub Flow for non-production environments as well. An example workflow with a develop environment could look something like:

  1. Create local feature branch (add/new-feature).
  2. Develop, test, commit changes.
  3. Push up changes to new remote branch.
  4. Create a PR from the remote add/new-feature branch to develop.
  5. (Optional and recommended) Internal team code review.
  6. Merge PR and test on develop environment.
  7. If everything looks good, create a PR from the remote add/new-feature branch to master.
  8. Proceed with VIP Code Review.

↑ Top ↑

My code is approved, how do I deploy? #

Once a member of the VIP team has approved your code, the next step is to merge your code to master which triggers this code to be deployed on the site. Typically, deployment takes less than ten seconds.

GitHub documentation on merging a pull request.

↑ Top ↑

Scheduled reviews for large changesets #

Large changesets and PRs take longer to review, and can be more complex. If a changeset is larger than about 1,000 lines of code it may need more time than our traditional review/deploy workflow facilitates, to give it the proper attention it deserves. In these cases we may mark your PR as [Status] VIP Scheduled Review, and set a date when we can assign an engineer to spend the required time to review your changes.

As an alternative to having a large changeset undergo a scheduled review, the PR could be broken up into smaller, more atomic commits. Then, a number of smaller PRs could be created, reducing the review time. One workflow for achieving this is:

  1. Create a fresh branch off master.
  2. Check out one of the changes from the branch used in the large PR.
  3. Push this change to the new branch.
  4. Commit and push up the new branch.
  5. Create a new PR.

More detail on this suggested workflow can be found here: https://www.bigeng.io/git-magic/

↑ Top ↑

Reminders for the speediest reviews #

Most pull requests can be approved more quickly if these guidelines are kept in mind during development:

  • Obtain internal reviews before opening a pull request on master; as explained above, you can do an internal review by targeting a different branch such as develop. The VIP bot will recommend changes on any pull request.
  • Only open a pull request on master after all development, internal reviews, and PHPCS fixes are complete.
  • Avoid making changes to an approved pull request – if possible make those in another branch, or wait until the changes are merged.
  • Separate pull requests should be employed for any of the following scenarios:
    • Extensive whitespace changes (for example, fixing tabs and spacing).
    • Changes that are limited to style only (for example, revising variable names, formatting, comment blocks, etc).
    • Addition of a plugin (one pull request per plugin is best).
    • Removal of plugins, or several files.
    • When multiple, distinct features are changing that, combined, affect more than 1000 lines of code or more than 50 files.
    • A build step that results in minified or compiled JS and CSS (also consider using our Built Branches).
  • Pull requests that contain only CSS changes (or do not contain files with extensions that we review) will be auto-approved.
  • When making fixes to a review that had Request Changes reviews, try to make only one or a few commits – this can be looked at separately
  • Be sure to dismiss an approval review if you need to make an additional change for any reason, and try to limit those changes to one commit, or merge the reviewed pull request and open a follow up one
  • Pull request commit messages and the initial comment are helpful to us and we do read them; summarise the intent of the changes and add detail about complex abstractions.
  • If your pull request is related to an open help desk ticket, please provide a link to that ticket in the comment.
  • Once a pull request is created, open it in Github and confirm there are no changes included that were not intended, especially if there were .gitignore file changes.

By creating separate pull requests for smaller changes, each review is simplified and can be completed more quickly. When there are many files, many PHPCS warnings or errors, or many commits after the initial review, it becomes more complex, and sometimes more difficult to use Github’s built-in tools to provide review feedback.

↑ Top ↑

Frequently Asked Questions #

↑ Top ↑

What happens when I merge? Are composer dependencies pulled in, etc? #

Any commit to the deployment branch (master, develop, etc), including a merge from a pull request, will result in a webhook causing the files in the branch to be updated on the running systems. No builds run during this process, so the files in the branch need to be ready to deploy.

If you do need builds to run, for example to run composer update or composer install, or to build or compile any other files such as sass, scss, and JavaScript, we recommend utilizing our built branches system, an automated build and deploy service that will, instead of deploying master, run your designated build script on a CI system, and if your checks and tests pass, update a master-built branch and deploy that. You can use this for any or all of your deployed branches and environments, and this option allows you to avoid any need to commit or review dependencies and build-generated assets.

↑ Top ↑

How do I get the VIP Bot to check my pull request? #

The bot will run automatically on any open pull request (targeting any branch) when the pull request is opened or following any new commit. Depending on the size of the change, it may take a few minutes for feedback to appear. If there are no issues in its automated scan, it will not add any review notes.

↑ Top ↑

Do I need to request review from VIP or the VIP Bot user? #

No. Requesting a review will not change our review process, and the bot does not respond to review requests or assignments. We’ll review the change if it’s against master and your repository is set up for reviews.

If you would like code reviewed, or advice on code, outside of this workflow or on a non-reviewed branch, please open a helpdesk ticket with a link to the changeset or pull request, or an embedded snippet for short changes, along with any questions and/or context that helps us provide feedback.

↑ Top ↑

What is the difference between the VIP Bot’s review and a VIP review? #

The VIP Bot runs PHPCS on the changes in the pull request and provides an automated way of obtaining that feedback. Typically, some feedback may need human review – by your team or by VIP – and may be found to be invalid, since many items flagged require an inspection for usage and context. VIP will also provide feedback or request changes on things that the bot is unable to detect, or things the bot flagged that have not been addressed but should be.

↑ Top ↑

How do I create a new theme (or plugin) from an existing one #

To streamline review for a brand new theme based on an existing theme, copy the theme to a new file tree and commit it without changes in a new branch.

Then open a pull request to merge to master, and note that it is a duplicate of (the original theme).

Once that is approved, make modifications and submit for review as normal. This allows VIP to review only the subsequent changes, and will speed up reviews.

↑ Top ↑

How do I create a Pull Request? #

GitHub has an excellent guide that covers this in detail: https://help.github.com/articles/creating-a-pull-request/

↑ Top ↑

How do I test a colleague’s PR branch on my local machine? #

You can create a local branch that “tracks” a remote branch. Find the branch name for your colleague’s PR then use the following to checkout the branch locally:

Let’s say the branch is called “add/new-feature”:

git checkout -t origin/add/new-feature

You can now test the changes in your local environment. You can also commit, push, and pull directly to/from the branch as well.

More details on tracking branches at http://gitready.com/beginner/2009/03/09/remote-tracking-branches.html

↑ Top ↑

I merged and deployed my PR. How do I get started on my next feature? #

Congratulations on your successful merge and deploy. Take a moment to celebrate, first! 🙂

Then:

# Switch back to master
git checkout master

# Get the latest changes
git pull origin master

# create a new branch and continue development

↑ Top ↑

Oh no! I committed to the master branch! #

We can fix this.

You first need to figure out how many commits you need to move over to the new branch. Let’s say it’s 3 commits.

# Create a new branch with your commits included
git checkout -b add/feature

# Switch back to master
git checkout master

# Delete the commits from the master branch
git reset --hard HEAD~3

# Go back to your new branch
git checkout add/feature

# Continue on with your development

As another example, if it was 1 commit, your reset command would change to git reset --hard HEAD~1

↑ Top ↑

There were some updates to master and now my PR branch is out of sync. How do I update my branch to get the latest version of the site’s code? #

# Switch to master
git checkout master

# Get the latest updates
git pull origin master

# Go back to your working branch
git checkout add/new-feature

# Rebase the changes from master into your branch
git rebase master

If you run into conflicts, GitHub has excellent resources on how to resolve them from the command line:
https://help.github.com/articles/resolving-merge-conflicts-after-a-git-rebase/
https://help.github.com/articles/resolving-a-merge-conflict-using-the-command-line/

If you get errors about fast-forwarded branches, you can force push to the branch to get your changes up to the remote branch.

git push --force origin add/new-feature

↑ Top ↑

Someone else pushed updates to my branch and now git is complaining when I try to push up my changes. #

# Pull the changes from the remote branch and rebase them locally
git pull --rebase origin add/new-feature

# Fix any conflicts that come up

# Force push to the remote branch
git push --force origin add/new-feature

↑ Top ↑

Git tools & resources #

↑ Top ↑

Guides #

↑ Top ↑

Apps #

Ready to get started?

Drop us a note.

No matter where you are in the planning process, we’re happy to help, and we’re actual humans here on the other side of the form. 👋 We’re here to discuss your challenges and plans, evaluate your existing resources or a potential partner, or even make some initial recommendations. And, of course, we’re here to help any time you’re in the market for some robust WordPress awesomeness.