• Daniel (they/them)

Design review basics in git for hardware


Overview

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).


Design review tab - hardware engineering
AllSpice Design Review (PR) tab

After you’ve created the new design review, select the from and to branches, noting that most systems use a right-to-left direction.


Merge branch selection - git for hardware platform
AllSpice merge branch selection

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.


Design review creation process in git for hardware platform
AllSpice new design review creation process

The design review still isn’t created yet. Fill out the title and description, and optionally add labels, milestones, and assignees.


design review form on hardware engineering platform
AllSpice new design review form

Click on Create Design Review to finally create the review.



design review creation - hardware checklist
AllSpice design review creation

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.



hardware collaboration labels
AllSpice's default labels

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 design 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 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

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

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.


Branch protection

To set up your repo to allow reviewers to block a review, go to repo→settings→branches and click Enable Branch Protection.


Enable branch protection
Repo 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.


Repo merge blocking in git for hardware platform
Repo merge blocking

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.


merge files button warning on git for hardware platform
Design review cautioning merging until official reviews are complete

Likewise if one of your reviewers has requested changes, the design review can be merged by an admin.

Design review merge button in git for hardware platform
Design review merge

Design review checklist

You can add a checklist to your design review that shows the progress of completion of the checklist. Simple 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 design review 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.


design review completed tasks
Design review with 2/4 tasks completed

You can create your own design review checklist from scratch ad-hoc, or you can place a template in .allspice/pull_request_template.md.


Read more in our previous post on this hardware design review checklist here.


hardware design review checklist
Default design review checklist

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.