You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
All changes to our codebase go through a Pull Request process, they require an approval before they can be included in our codebase. At the moment there's a loophole in that system: after an approval is given changes can be made and included in our codebase without requiring a new review.
Within the internal team this hasn't been a problem because we've unofficially checked with each other before merging. This unofficial process has a cognitive overhead and would need to be taught to new team members.
If there were a bad actor trying to get malicious code into the kit then this loophole would be a good way of getting it in.
Github has an option to require a re-review in this scenario and I believe we should turn it on because:
It automates the process rather than relying on people to remember
The new team members won't need to learn the unwritten rule
It protects against malicious actions by bad actors
Who needs to work on this
Github admin (e.g. Natalie)
Who needs to review this
Done when
An approved pull request becomes unapproved when a change is made in:
govuk-prototype-kit
govuk-prototype-kit-docs
govuk-prototype-kit-step-by-step
govuk-prototype-kit-task-list
govuk-prototype-kit-common-templates
The text was updated successfully, but these errors were encountered:
What
If a change is made after a PR is approved the approval should be removed.
The instructions to make the change are well summarised on Stack Overflow with official github documentation also available.
Why
All changes to our codebase go through a Pull Request process, they require an approval before they can be included in our codebase. At the moment there's a loophole in that system: after an approval is given changes can be made and included in our codebase without requiring a new review.
Within the internal team this hasn't been a problem because we've unofficially checked with each other before merging. This unofficial process has a cognitive overhead and would need to be taught to new team members.
If there were a bad actor trying to get malicious code into the kit then this loophole would be a good way of getting it in.
Github has an option to require a re-review in this scenario and I believe we should turn it on because:
Who needs to work on this
Who needs to review this
Done when
An approved pull request becomes unapproved when a change is made in:
govuk-prototype-kit
govuk-prototype-kit-docs
govuk-prototype-kit-step-by-step
govuk-prototype-kit-task-list
govuk-prototype-kit-common-templates
The text was updated successfully, but these errors were encountered: