Design review best practices

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.

Traditional reviews have a monofocus. The downside to this is 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 offering their own review actions. It also follows along the narration of the presenter, meaning if someone has a particular speciality 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

Design review merges committed 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.

Changes can continue to be pushed to a review branch until the review is complete. 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.

Creating design reviews is easy. 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).

AllSpice Design Review (PR) tab

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

AllSpice merge branch selection

Review your commits. 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.

![][image11]

AllSpice new design review creation process

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

AllSpice new design review form

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.

AllSpice’s default labels

Use as many labels as you think apply. You can always remove them. 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

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 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 the design review is completed.

Assignees

Assignees are the people who are responsible for managing the design review. Assignees are different from reviewers, although they sound similar. They 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 the people who are responsible for looking at your file changes, milestones, and design review description and determining 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.

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

You can create your own design review checklist from scratch ad-hoc, or you can place a template in .allspice/pull_request_template.md. If you want a copy of AllSpice’s template, they keep a copy of it here: Design Review and Issue Template Demo

Here are instructions on how to create and setup your own design-review templates.

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.