Overview
Every hardware engineer is familiar with traditional design review. In the past, it made sense to gather a group of people in a room, staring at a projector screen, and manually walk through the schematics and PCB layout. Every person has a chance to voice their input and create action items.
The downside to this is that reviews could only happen during time slots when everyone is available. It also only allows for one person to comment at a time, preventing others to look through the design and offer their own review actions. It also follows along the narration of the presenter, meaning if someone has a particular specialty or is interested in a specific sub-circuit, they have to wait until that part of the walkthrough. This means that if the narrator doesn’t cover something, it can get missed. Old-style reviews are useful, but they leave a lot of potential bugs and stones unturned.
Git design reviews, also known as a “pull request”, allow for asynchronous reviews. You can still have a formal meeting and track changes externally, but before everyone meets, you have a chance to circulate the design. Many of the design issues can be resolved before stepping foot in a meeting room. Because the review is virtual, you can invite more people to the meeting. Domain experts like mechanical, manufacturing, and test engineers can be invited to more reviews and not worry about schedule conflicts. Reviewers also get more time to review and get to work at their own pace. Instead of many people sharing an hour or two, with mono focus, each team member can review the features of the design where they are experts and defer to others’ expertise on sub-circuits where they are not.
Design review basics
At its core, a design reviews merge changes from one branch into another branch. Because of the problem with merging ECAD files, we recommend using a two-branch strategy like main and dev. In this case, a review would merge changes from the dev branch into the main branch. You can also read our chapter on branching strategies for more flexibility in the full eBook.
When you create design reviews, anyone with access can continue to commit changes to that branch, and the new changes will be merged when it is complete. This is how designs get fixed during the review. If you want new changes to be added to the repo, but aren’t part of the design review, you must create a separate feature branch. Keep in mind that creating changes in parallel with your review means that the new features will have to be manually updated with the changes from the reviewed branch after the review is complete.
The easiest way to create a design review is through the web interface. Navigate to your repo, click on the Design Reviews (Pull Request) tab, and click New Design Review (New Pull Request).
After you’ve created the new design review, select the from and to branches, noting that most systems use a right-to-left direction.
The design review isn’t created yet. The system will show you a list of commits that will be merged. After you have reviewed the changes, you can click New Design Review to move to the next step.
The design review still isn’t created yet. Fill out the title and description, and optionally add labels, milestones, and assignees.
Click on Create Design Review to finally create the review.
Labels
Labels are like categories and are re-used throughout the life of your repo. You can add labels to a design review, issues, or milestones and the same labels are shared among them.
This is a list of the default issues included in an AllSpice repo.
You can use more than one issue, and it’s often handy to mix things like level of importance along with the types of changes inside the review. AllSpice uses labels like bug, DFM, feature, mechanical, and layout to let people know what type of review is happening.
Labels are customizable, and you can add as many of them as you need to match your existing process.
Milestones
Milestones are collections of issues and help let people know what is being fixed or changed in all of the committed changes. Note that when you close the design review, the issues on the milestone will not automatically close. Milestones are usually created before the work is committed, but there is nothing wrong with creating a milestone at the last minute and adding the issues to help organize your project management.
Closing issues
To close the issues when the review is complete, add them to the design review description with one of the following keywords followed by the issue number.
- close/closes/closed
- fix/fixes/fixed
- resolve/resolves/resolved
Examples:
closes #47
fixed #132
resolve #888
Dependency to issues
You can add issues that prevent the design review from completing until they are closed by adding them to the dependency list. Any issues that are open will prevent it from merging with an error message. This is different from adding issues in the design review description, as that lets you close issues when you merge the files in the design review. This is also different from milestones, which do not need to be closed and will not be closed when it is completed.
Assignees
Assignees are different from reviewers, although they sound similar. They are the people who are responsible for managing the design review and have full rights and access to the review. One person can create the design review and assign it to another person or group of people who are fully responsible for completing the review. You can also assign someone and co-manage the review, with all assignees having equal permissions as the creator.
Reviewers
Reviewers are pretty straightforward. They are the people who are responsible for looking at your file changes, milestones, and design review description to determine if the committed changes are ready for merging, or if additional changes need to be added. All reviewers must be collaborators on the repo. If you find that you can’t add any reviewers, it might be because there are no collaborators set up.
Reviewers can comment, approve, or request changes. By default, if a reviewer requests changes, there is nothing stopping an assignee or the creator of the design review from merging the changes anyways. Likewise, the design review can be merged regardless if any or all of the reviewers approve. It is unwise to ignore the approval or requested changes of a reviewer. After all, that is the whole point of the review, to solicit the advice of others and integrate their wisdom and experience into your design. If you are going to merge without approval, or after someone has requested changes, it is best to leave a comment as to why.
Branch protection
To set up your repo to allow reviewers to block a review, go to repo→settings→branches and click Enable Branch Protection.
Select “Block merge on rejected reviews” to prevent the merge from completing when someone requests changes. Select “Block merge on official review requests” if you have reviewers who have not approved the design review.
If you are an admin to the repo, you can still merge the design review, regardless if there are official review requests. The merge button will turn red to indicate that only admins can merge the files.
Likewise, if one of your reviewers has requested changes, the design review can be merged by an admin.
Design review checklist
You can add a checklist to your design review that shows the progress of completion of the checklist. Simply add “- [ ]” to indicate a line item that is not complete. Add “- [x]” to indicate an item that is completed. You may also manually click on the checkboxes to mark them as completed or uncompleted.
When viewing the summary, you can see how many steps of the checklist have been completed. You can see here that 2/4 of the checklists have been completed.
You can create your own checklist from scratch ad-hoc, or you can place a template in .allspice/pull_request_template.md.
For more on this, read our hardware design review checklist here.
Summary
Git design reviews (pull requests) offer very powerful tools to help you and your team review your designs. They are inherently asynchronous, and allow for much more productivity when reviewing changes. It is worthwhile to explore all of the features and to see how they fit into and augment your current review workflow.
