The purpose of this document is to help development teams associated with the University of Illinois fulfill their responsibility to comply with Illinois Cybersecurity standards, including IT-7, IT08, and IT13.

Use this document to guide discussions on your development team of adopting effective code review practices. Teams are encouraged to use this document as a starting point to draft a more detailed document internal to their team, committed to by all team members.

TL;DR:

• Request lots of reviewers.
• Use the bot.
• Expect fast feedback.

• Submit pull requests often. Try to keep each pull request to 20 lines of new non-boilerplate code or less.
• Link relevant GitHub issues to the pull request.
• We typically use Github commit keywords to link issues. This adds the issue history to our Git log.
• Request lots of reviewers: No one sees a pull request until reviewers are assigned.
• Feel free to request reviewers from other teams.
• Teams commit to providing additional reviewers when requested by pull request.
• Add the GitHub Teams reminder bot to all repositories.
• You are encouraged to remind colleagues of your open pull request.
• Be available in chat to discuss the code.
• Always mention any ready-for-review un-reviewed code during your daily stand-up.
• Teams should commit to reviewing un-reviewed code daily.
• Some feedback merits opening a new issue, to keep the current pull request simple.

• We prioritize reviewing each other's code, over writing new code.
• We encourage having a look at any open pull requests assigned to you each time you return from a coffee break.
• One work day is the longest we expect any non-draft pull request to sit without feedback.
• Please do review code you don't fully understand.
• Your manager will make time for you to train in languages that you are being asked to review code in.
• Pair and mob programming count as code review.
• Feel free to approve pull requests for work that you are satisfied with after any collaborative programming session.
• Review does not result in automatic approval, even when the work was done in a mob session.

• Merging a pull request requires 2 approvals.
• We have disabled the default create a merge commit, because we require a linear history. This saves headaches for everyone when merging.
• We prefer squash and merge as the typical method to merge to main. This results in one commit per pull request.

The following GitHub repository settings are recommended for fast effective code-review:

• Require pull request reviews before merging: two approvals is a recommended starting setting.
• Restrict who can push to matching branches: use this setting to ensure only pull requests update the default branch.
• Require status checks before merging: use this to ensure that tests, static code analysis and secret detection pass before merging each pull request.
• Require linear history: linear history simplifies merging.
• Require conversation resolution before merging
• Use a pre-commit hook to prevent checking in secrets

These resources are tailored for use with the University of Illinois Shared GitHub Service, but most of these principles can be applied to any source control solution.

• Linking a GitHub pull request to an issue using a keyword
• What's the Difference Between the 3 Github Merge Methods?
• Use a pre-commit hook to prevent checking in secrets