Skip to content

Our way of working with PRs and peer‐reviewing

Tom edited this page Dec 30, 2023 · 1 revision
  • Work from branches; On all main repositories (i.e. the beta and module ones for USB and Smile respectively) have 'main branch protection'. Meaning you should not commit to main. (We didn't fully restrict ourselves, so you can, just don't. Also pre-commit won't let you proceed without having a local branch).
  • Generate a meaningful commit message each time (yeah, we all do the Oops, typo one 3 times again, but we all know we should not).
  • When creating your PR (see above on 'how git works') - the first time you push your branch your tool of choice will generate a PR link for you. If you missed that, just on GitHub go to the 'branch' page and select your branch, it will suggest creating a PR.
  • Create your PR but refrain from being overly complete; ensure the Title is correct at minimum, though @CoMPaTech really likes it if you set the labels and project so we keep an overview. Unless you are really confident, please set to Draft. (The create button has a little ^ on it to toggle between draft and real PR).
  • Wait for all the actions to complete; sometimes the cache fails. Either bump the cache version in the appropriate .github/workflows/.... file - or go to the actions tab; lower left corner should show the active caches, just delete them. (It will take longer to reprocess your action).
  • Wait for a peer-review. When setting to 'ready' it automatically invites team-members to review (by mail). That or if it's a small change or nobody's available, that is why we didn't fully protect, you can merge yourself.
  • Mostly we tend to 'merge' our own PRs (i.e. not merge as a reviewer), though that is just what @bouwew and @CoMPaTech sort of did together. It doesn't really matter (but as a reviewer you might forget that the contributor first wants an opinion before bumping the release version) :)
  • When merging to main, don't forget to release (see elsewhere) when relevant.
Clone this wiki locally