Everyone of you is most likely familiar with Pull Requests (PRs) on GitHub. They are an essential tool for collaboration on projects - be it open source or commercial ones. As such they are a key utility for our daily work at collaboration Factory AG, especially concerning the development of our core framework - the platform cplace.
Disclaimer: Since we had some issues at the beginning with how to best deal with PRs in our development process I want to describe the way we handle them currently. This post therefore is an opinionated and maybe flawed way of managing them - so if you have any suggestions for improvement just leave a comment below and I'll be happy to follow up.
The original Problem
As mentioned above PRs are one of the key utilities to manage changes to the core part of cplace - the so-called platform. Basically every change needs to be reviewed by at least one other team member in order to increase quality and reduce error rates. GitHub provides a very nice and practical UI as well as the tools necessary to efficiently deal with PRs: review, comment, discuss, and finally close them.
The main issue we had however was an ever growing list of outstanding PRs. States were not clear, i.e. it was not immediately visible which were awaiting to be reviewed, who was responsible for asking for help or closing the ones already completed. As a result we had some dead PRs lingering around for months and cycle times measuring the duration from opening to closing were horribly high.
We therefore set out to tackle this as the main issue: reducing cycle times of PRs in order to make new features and bugfixes available to customers and partner developers quickly while maintaining high standards.
The Labeled Lifecycle
One of the features GitHub provides was used by us only to a marginal extent - labels. However, they are easily filterable and allow to quickly categorise PRs. Our new workflow consequently makes good use of them. Let‘s take a look at the complete lifecycle of a PR as shown in the image below.
1. Creating the PR
When work on the PR starts it is initially created with the label
working. The Assignee of the PR is set to and will always stay its original creator. As such the responsibility of working, tracking, and following up on the PR is defined clearly.
2. Requesting Reviews
Once the implementation has been finished the creator sets the label to
needs review. Thereby others know which PRs are awaiting another pair of eyes for cross-checking. We don‘t really make use of the Reviewers on the other hand since we want to encourage all of our developers to review as much as they like and especially from all areas of the system, i.e. not limited to their special domain. This boosts confidence and knowledge across the whole platform.
3. Conducting a Review
Developers pick any PR labelled
needs review and make a thorough review of the changes. As a result the PR can be either approved or changes requested. We do not use comment as the outcome of a review which I will explain further on.
Approval is the easiest workflow - what remains for the reviewer is to set the PR‘s label to
ready to merge. The assignee - i.e. the original creator - thereby knows the review has been conducted and completed successfully.
If the reviewer on the other hand has any remarks he marks his review as changes requested and assigns once again the label
working. Every developer then can filter all PRs with the label
working and state
changes requested to quickly find the ones which require additional effort and changes by him. We decided to ditch comment as review result in order to make clear that an additional discussion is required - we further add an additional
question label (not shown in the image) to these PRs.
4. Follow Up Work
Every developer checks his PRs according to their states. The ones with
ready to merge can now be immediately merge into their destination branch adding a proper detailed commit message to be used for changelog generation. The only exception is if the creator wants other developers to do an additional review - in this case he resets the label to
needs review and awaits another review.
All PRs which are labelled
working and have changes requested need to be tackled as soon as possible and the required changes must be completed in a timely manner. Once these are done he changes the label back to
needs review as described in step 2.
No Improvement without Process
Of course changing and deciding on a new workflow did not immediately change anything by itself. They key point was to introduce it as process or discipline in the daily workload.
We agreed to dedicate two separate time boxes per day of 30 minutes each to reviewing and follow up work - directly at the beginning of a work day as well as after lunch time.
The priorities are clearly defined as follows:
- Merge when possible - Check if you have any PRs
ready to merge. Decide whether to request additional reviews or otherwise immediately merge the completed feature / bug / etc.
- Follow up work - first everyone has to check whether there are any of his PRs in
workingwith changes requested. If so take the time box to work on those.
- Review - when there‘s time left pick any PR with
needs reviewand start your review. Here PRs you already reviewed before and had re-work done have priority over completely fresh ones.
By better leveraging the tools provided by GitHub and introducing our own lifecycle we could greatly improve cycle times for PRs. There are hardly any - yes, sometimes they still exist - PRs which await completion longer than a few days. It also helped to keep the number of open PRs down and constant at around 20 to 25. In combination with the assignment of Milestones we are now able to prioritise and focus our work on what is necessary.
Are you using your own labels for lifecycle and workflow management of PRs on GitHub or are you solely relying on the standard review states (approved, changes requested, commented)? Let me know if you have any questions or suggestions for improvement in the comments below and I will follow up as soon as possible.