Design review basics in git for hardware
Every hardware engineer is familiar with traditional design reviews. In the past, it made a lot of sense to gather a group of people in a room, stare 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 or as many people as possible are available. It also only allows for one person to comment at a time, and while they are commenting, no one else can be looking 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. It also 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 review merges 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 design review would merge changes from the dev branch into the main branch. You can also read our chapter on branching strategies for more flexibility.
When you create a design review, anyone with access can continue to commit changes to that branch, and the new changes will be merged when the design review 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 design 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 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 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.
To close the issues when the design review is complete, add them to the design review description with one of the following keywords followed by the issue number.
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 the design review from merging with an error message. This is different than 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 the design review is completed.
Assignees are different than reviewers, although they sound similars. Assignees 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 are pretty straightforward. They are the people who are responsible for looking at your file changes, milestones, and design review description and 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.
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.