Developer Workflow

Hyperion tracks upcoming work through github issues and uses the Hyperion project board to manage in progress work using the following workflow (note that the workflow below includes the command line git instructions, if you are using VSCode it may be easier to use the integrated VCS):

1. An issue is written describing a piece of work. This should be as clear as possible, ideally with measurable Acceptance Criteria. Development work should not be done without an issue describing it.
2. At the sprint meetings issues are prioritised and it is collaboratively decided what to work on for the following sprint, these will be put in the To Do column of the project board. If an issue is urgent it can be pulled into the sprint with the agreement of one other developer. Outside collaborators (e.g. those not in the MX DAQ team) are free to work on issues of their own choosing without them being prioritised this way.
3. A developer can pick any of the issues out of the To Do column to work on, when they do this they should assign the issue to themselves.
4. Developers should then branch off main to do the work, the branch name should be along the lines XX_description where XX is the issue number and description is a brief snake case ticket description. You can create a branch with git checkout -b XX_description.
5. Where changes are also required in dodal, developers should create a branch in dodal with the name hyperion_XX_description, where XX and description matches that of the Hyperion branch.
6. Developers then add code to solve the issue. They can let git know about the code additions by running git add -u. Every now again you should commit new work to git, this is like doing a save. You can do this by doing git commit -m "(#XX) Description of the commit" (where XX is the issue number).
7. Push your changes to Github by running git push --set-upstream origin XX_description (where XX_description is the branch name, note that the console will tab-complete the branch name for you)
8. When the developer is happy with the work they should make a PR merging the branch. The developer should also create a PR in the dodal repository if they have made changes in dodal. The PR(s) should have the following:
   • The issue number in the title
   • Fixes #XX in the description, where XX is the issue number. For the dodal PR, this should link to the hyperion issue.
   • A description for the reviewer on how to test the change
   • If changes in dodal have been made, the hyperion PR should link to the dodal PR. Additionally, setup.cfg in the hyperion root folder should be changed: in the line dodal @ git+https://github.com/DiamondLightSource/python-dodal.git@hash, change the hash to that of the relevant dodal commit.
9. The developer should then pick another issue from the review column to review (You can find issues that need review by using is:pr is:open review:required -author:XXXX in the PR search box where XXXX is your GH username
10. When reviewing you should ensure the code meets the coding standards. If you have comments for the reviewer you could add them with the following prefixes to be helpful:
    • Nit - This is arguably just taste, you don't mind if the developer doesn't follow this
    • Should - This will improve code quality but unlikely to affect functionality. If the developer doesn't want to do this they should write an explanation.
    • Must - This will negatively affect functionality if not implemented.
11. If the review passes then the reviewer should approve the change and merge it
12. If the review fails the reviewer should request any changes (this issue is said to require rework)
13. A developer should ideally pick up any rework PRs before starting new work
14. When a developer is happy that a rework is complete they should press the re-request review button on the PR

Modifying Code as Part of a Review

Reviewers should feel free to make small changes to a PR as part of a review e.g. typos, poorly named variables, fixing a test. However, if code has been changed in this way they need then need to run it past another developer (ideally the original developer). This means that no code is merged into main without being looked at by at least 2 people, ensuring collective responsibility for the codebase.

Dismissing a previous reviewer

There may be times where one person requests changes on a PR, these changes are addressed and another person then approves it. In this case the PR still cannot be merged until the original review is dismissed. We're happy for people to dismiss stale reviews with the understanding that:

• The new reviewer should check that the must and should comments have been addressed
• If it's a big change it may still be worth the original reviewer looking again, if so they should make this clear to the original reviewer and the person that wrote the code