SOP for feature branch development workflow

[joeflack4]

Overview

@twhetzel recently decided on #404. WIth that comes a new development workflow.

Sub-tasks

• [ ] 1. Formulate the SOP @twhetzel @matentzn
• [ ] 2. Finalize and upload somewhere @joeflack4

1. Formulate the SOP

Standard development workflow

1. Create feature branch: Create new feature branch off of develop and open PR
2. Code locally: Get code / tests to pass in development environment / debugger
3. Run workflow in ODK: sh run.sh make ...
4. Build
   • Run build, e.g. sh run.sh make build-mondo-ingest -B
   • Add some build results / details to: Mondo ingest build stats
   • Build PR: Create a new branch / PR off of the feature branch.
   • When opening PR, use feature branch as the base.
   • Leave it in draft mode.
   • Prepend DO NOT MERGE - to the PR title. Otherwise a good title for the PR would be an exact copy of the feature branch's title, or similar. And ideally include the word 'build' somewhere.
   • When build is approved, close its PR.
5. Merge when approved
   • If there are not outstanding issues in the feature branch PR or its build PR, and necessary number of reviewers (usually 2, sometimes 1) have approved, it then can be merged into develop
   • Optional?: Run the build again after right after merging into develop? If so, merge any new data file updates?

Additional possible steps: 1 PR for code, & 1 PR for code + outputs? Trish and I were thinking about another workflow that might make sense mondo-ingest / ontology repos beyond the normal git flow (main <- develop <- feature branches). Basically, there PRs can be big and hard to read because they are a mix of code (normally just a few files) and outputs (many files). It might make PRs easier to review if we had a develop -> feature branch 1 just for code, and then feature branch 1 -> feature branch 2 for code + outputs. Although instead of doing this, perhaps file extension filters in the files changed section of a PR could be enough.

Perhaps will want this SOP to go on the same page as this: https://oboacademy.github.io/obook/howto/review-pull-request/

Hot fixes

Sometimes, code changes will be necessary to apply directly to main in order to fix bugs so that builds can run. This skips the normal development workflow.

Additional guidelines

• "3-word commit headers"

• 544

• Approvers: At least 1 approver who is not the PR author is required.
  • In the case of (5), this requires 3 approvals: (i) the build on develop, (ii) the merge into main, (iii) the build on main
• Build PRs for feature branches should be left in draft mode, as they are not to be merged.

2. Finalize and upload somewhere

I didn't see an appropriate file for this currently in docs/. How about docs/developer/sops.md or docs/developer/sop_development_workflow.md.

Related

• Mondo release cycle
• Mondo ingest development and build cycle

[matentzn]

Great initiative to get this started.

I would also suggest that even minor (non-functional) PRs (eg this should get a review from now on.. We didnt do this so far, but at least until we have our development SOP sorted, maybe its a good way to ensure knowledge transfer (@twhetzel).

[joeflack4]

@twhetzel I added a step (5) to the workflow for merging develop --> main.

The other day, we were merging in ICD11 and I had to run the build twice. But I've proposed a variation that I think is smarter; it will require that the build only be run once. Can discuss at meeting.

So the new proposal is:

1. main -(create)--> main-temp
2. develop -(merge)--> main-temp
3. run build & commit files
4. main-temp -(PR)--> main

edit: Tried this workflow out in this PR:

• 554

And Nico and Trish already looked at the way I did this, but want to instead do as this SOP describes, which is: build on develop, merge into main, and build again.

[twhetzel]

To follow-up here at meeting, let's keep with the original plan to run the build on both the feature and develop branch, and then merge only the code changes into main.

[joeflack4]

Just following up on some previous meeting notes; I don't think I jotted down well enough, but I think the crux of it was:

• As soon as a build on main passes (runs without fail), we can merge it without review, and just notify on Slack.

[twhetzel]

Another note on merging develop into main is to do this when a new pipeline is complete, e.g. synonym sync, new resource like ICD11 Foundation, etc.

[joeflack4]

@twhetzel For convention on drafts, do we want to undraft when (a) all requirements for the PR have been completed to the author's understanding, but the build has not yet been run / passed, or (b) build has also been run/passed.

I think I've been doing / prefer 'a', because it lets people know 'this is ready to review', even if we don't have word on the build yet.

[twhetzel]

This development workflow should only apply to new features and managing feature branches since there is already a SOP in place for managing code in this repo related to the Mondo release cycle.

[joeflack4]

OK that sounds good to dedupe this SOP. I think it came out around the same time but now the Mondo release cycle is very fleshed out. And there's also Mondo ingest development and build cycle.

Renamed issue to 'SOP for feature branch development workflow'. Deletedmain and develop parts from the. Also added a step related to 'build stat's google doc. And added links to Mondo and mondo-ingest build cycles to 'related'.