b4b‐dev branch genesis - ESCOMP/CTSM GitHub Wiki

Introduction

We think there are some improvements that can be made to our current system of making tags for CTSM.

Problems to solve

  • Tie up work quicker
  • Tie up work before moving to something else (remove context switching cost)
  • Allow work to be used by others quicker
  • Prevent PRs from hanging for too long
  • The bottleneck of only having one next PR to be done

Good tagging habits to reinforce

  • Stages: Requirements, plan, design, implement, debug, test, clean, review, release
  • Plan the content, scope and purpose for the tag
  • Develop to the planned changes
  • Bring in just those changes
  • Make sure those changes are well tested
  • Small enough changes that code review is reasonable
  • Do in a short cycle without distractions
  • Keeping CTSM tags used by the community clean of major bugs

Proposal

  • Create a development branch that some (or all) PRs go into before finally getting merged into master
  • Master would still be the only place that makes tags
    • CESM would only use tags on master
    • Full test suites required to merge to master
    • Doesn’t really change for most scientists and users
  • Dev branch:
    • Small PRs (usually B4B) could go here, merged with others
    • Larger PR’s would go directly to master (and moved over to the dev branch)
  • Testing:
    • Dev branch has less testing requirements, so onus is smaller to get changes in
    • But these PRs still go through master testing eventually
  • Timeline:
    • Every 2 weeks merge everyone on dev into master (or sooner if something comes up)
    • “Release meeting” every 2 weeks (this is similar to Sprint planning for SCRUM)
      • Similar to scrums
      • Planning time for what you plan to get in (plus maybe some random extra stuff)
      • Meet with PR authors if problems show up in testing
      • Ending period when you make a tag
    • Rotates who leads
      • Leads initial planning meeting
      • Lead kicks off testing in AM of release day
      • Go through fails with group
      • If we can’t figure out a failing PR in XXX time, it gets kicked back to original author to fix for next round
      • Lead makes tag on master
  • Ejection: If we decide this is not helpful we can drop it
  • Cons:
    • Two branches to maintain
  • Pros:
    • Faster to merge in small PRs
    • But master is unbroken
    • Only do thorough testing every 2 weeks
    • No “queue”
    • Take the pressure off the schedule for new major pushes to the master, coinciding with significant releases. allow for new features that will eventually appear in the master to be more thoroughly community-tested by people using the dev. branch for research in the meantime.

Why do we have the current system?

  • Two people making tags looked different
  • Erik would bundle things together to limit full testing..

Glossary

  • Dev Merger: The person who merges dev into master (and then vice versa).
  • PR integrator: Someone responsible for a PR that goes into either master or dev.
  • PR Reviewer: Reviews a PR to come to master or b4b-dev

Questions

  1. ✅ How do we start new branches? Via master? Via b4b-dev? What about external collaborators?
    1. No external contributors (excluding SE Meeting attendees like Greg and Ryan) should touch dev branch. Use the usual process.
    2. If you’re pretty sure changes will be bit-for-bit, start with b4b-dev. If prelim testing shows non-bfb, rebase to master to your branch (create PR direct to master). You could also make a new branch from master and cherry-pick your changes. The idea is you want to get rid of everything from the b4b-dev branch.
    3. When PRs get merged to master they should get merged to dev (i.e. dev shouldn’t be behind master). No. b4b-dev is allowed to get behind master. Dev Merger, after merging dev into master, merges master back into b4b-dev.
    4. Job of Dev Merger is to merge b4b-dev into master (and then vice versa). PR integrator merges straight to b4b-dev, with a PR of course.
      1. This should remove the onus of coordinating merges to b4b-dev.
      2. Dev Merger always tests b4b-dev HEAD against latest master tag before merging into master, regardless of whether there are merge conflicts.
    5. Jamboard: https://jamboard.google.com/d/1bK4NIogcLuhCmn7TQI2Vnnp6fEItCXg9LbqFVhGAfg4/viewer?mtt=87plbm3f8am9&f=0
  2. ✅ How often do we merge to master? Every 2 weeks, at least to start.
  3. ✅ How much time do we spend fixing errors each week/time before calling it that go-through.
    1. No more than a few minutes. If Dev Merger can’t figure it out, communicate to the PR integrator that there’s an issue and their PR will be kicked out of the merge.
  4. ✅ How do we communicate this process with others? Who else needs to know?
    1. Just people who come to this meeting need to know. We can move external contributors’ PRs to b4b-dev if we want to.
    2. Consider talking at CSEG meeting about our experience once we’ve implemented this for a while.
  5. ✅ If something gets pushed to master (e.g., an external collaborator makes a PR) whose responsibility is it to move it to b4b-dev, etc.? Or do we need to do this?
    1. Big external or internal PRs (e.g., CN matrix) probably wouldn’t go to b4b-dev first, just be its own PR to master?
    2. Dev Merger does this immediately after committing to master.
  6. ✅ Continuous integration would be awesome (nightly testing) - but we need so much compute time for this!
    1. Could do python testing nightly?
    2. Chris Fischer does nightly testing - cron job
    3. Not dealing with this now.
  7. ✅ Mini test list for individual PRs to b4b-dev?
    1. Suggested tests if you don’t already know what you want to use
    2. 2024-01-04: No; all b4b-dev PRs get full testing.
  8. ✅ Does documentation go to b4b-dev or directly to master?
    1. Typo in comment?
    2. Documentation and comment updates are exactly the kind of bit-for-bit changes that are perfect for going into the b4b-dev branch.
  9. ✅ Does everything go through a PR to b4b-dev? Or go directly into b4b-dev?
    1. Yes; see 1-d-iii above.
  10. ✅ Do we need someone to approve PR to b4b-dev?
    1. Yes, maybe with the exception of pure doc update PRs.
  11. ✅ How do we handle FATES-related PRs?
    1. Default parameter file only API updates
      1. These typically include fates science updates that make them non-B4B.
      2. Relatively small code changes
      3. Having these associated with a ctsm b4b-dev tag is helpful for fates devs
    2. Bundling multiple fates API breaking changes into a single probably should be avoided.
    3. Same rules apply. Anything that is not bit-for-bit goes directly to master instead of b4b-dev.
  12. ✅ Only non-answer-changing? Yes.
  13. Consensus is that initially we would only have b4b PRs to the b4b-dev branch.
  14. ✅ How to handle ChangeLog updates?
    1. Suggest that each merged feature branch should include a new file, doc/ChangeLog.pr-number, with the ChangeLog template filled out for the feature branch / PR in question.
    2. The person doing the merge of b4b-dev to master will go through those, write the entry in doc/ChangeLog, and delete the doc/ChangeLog.pr-number files.
    3. Have a special PR template for PRs going to b4b-dev
  15. ✅ Do we put b4b-dev branch PRs in Upcoming Tags project?
    1. No. Just have a recurring card with the date and assignee (Dev Merger) for the next regular merge to master.
    2. The thing that is planned around is the merge tag
  16. ✅ Do we allow new science to come in on b4b-dev? E.g., new options that don’t change answers under default settings.
    1. No.
  17. ✅ Do we allow new tests to come in in b4b-dev?
    1. Yes.
  18. ✅ Sometimes PRs need to be removed from a merge tag or from the bfb-dev branch. git revert seems to sometimes work and not close the reverted PR. How to handle this?
    1. If a merge tag accidentally closes a PR, PR Author should open a new one (and link back to the original one).
  19. ✅ Erik: For the b4b_dev branch suggest requiring PR’s for master status checks to pass and conversations to resolve. But, b4b_dev could allow direct pushes to master. See settings options for branches (link might only work for admins or Erik).
    1. Add rules to master: Require a PR before merging, etc.
    2. Do the same for b4b-dev.
  20. ✅Do we allow new history variables to come in on b4b-dev?
    1. Yes

Resources