Thoughts on merging vs rebasing

Potential change to git workflow: instead of running git merge master from a branch checkout, use git rebase -i master; this re-applies all commits (in order) to the current head of master. Combined with git merge branch --no-ff, this produces a more informative git graph. The following compares two ways of bringing in the four current open pull requests (in both graphs, time is increasing from bottom to top):

MERGING

*   a732fc0 (HEAD -> master) Merge branch 'enhancment/biomass_lim_factors'
|\
| *   9a762c7 (enhancment/biomass_lim_factors) Merge branch 'master' into enhancment/biomass_lim_factors
| |\
| |/
|/|
* |   7ba7e24 Merge branch 'bugfix/248'
|\ \
| * \   d899494 (bugfix/248) Merge branch 'master' into bugfix/248
| |\ \
| |/ /
|/| |
* | |   5581342 Merge branch 'bugfix/255'
|\ \ \
| * \ \   75f38fc (bugfix/255) Merge branch 'master' into bugfix/255
| |\ \ \
| |/ / /
|/| | |
* | | |   9effcfb Merge branch 'bugfix/244'
|\ \ \ \
| * \ \ \   ba7b8b6 (bugfix/244) Merge branch 'master' into bugfix/244
| |\ \ \ \
| |/ / / /
|/| | | |
| * | | | 18a287e (origin/bugfix/244) Rename zoo1[34]C to zoo1[34]Ctot
| | * | | a254387 (origin/bugfix/255) Add reference to Jahn, et al. 2015
| |/ / /
|/| | |
| | * | 7f96122 (origin/bugfix/248) Initialize data in surface_forcing_internal to 0
| | * | a0cea25 Move if (lcompute_nhx_surface_emis) line
| |/ /
| | * 278b2b1 post-code review cleanup (all yaml/json)
| | * de6c62d correct units in settings metadata
| | * c0ae482 use temperature units (degC, K) consistently
| | * 0ff947d correct units on some diagnostics
| | * e7712d1 change (non-vertical average) limitation diagnostics to surface only
| | * 74dc9cf add diagnostics: vertical average over 0-100m, weighted by carbon biomass, of limitation terms
| |/
|/|
* |   21f77a2 (tag: marbl0.28.5, origin/master, origin/HEAD) Merge remote-tracking branch 'klindsay/bugfix/253'
|\ \
| |/
|/|
| * b986d61 updated default_settings values from jkmoore
|/
* 5ce7fe9 (tag: marbl0.28.4) Bugfix: indices of diag%field_3d were wrong

REBASING

*   3633890 (HEAD -> master) Merge branch 'enhancment/biomass_lim_factors'
|\
| * 79e677b (enhancment/biomass_lim_factors) post-code review cleanup (all yaml/json)
| * 9dcfaf8 correct units in settings metadata
| * 1669619 use temperature units (degC, K) consistently
| * 1447c0a correct units on some diagnostics
| * e2615b5 change (non-vertical average) limitation diagnostics to surface only
| * 46e8be2 add diagnostics: vertical average over 0-100m, weighted by carbon biomass, of limitation terms
|/
*   91bb572 Merge branch 'bugfix/248'
|\
| * ce2b4af (bugfix/248) Initialize data in surface_forcing_internal to 0
| * 227a451 Move if (lcompute_nhx_surface_emis) line
|/
*   43e2a6a Merge branch 'bugfix/255'
|\
| * 5dffc39 (bugfix/255) Add reference to Jahn, et al. 2015
|/
*   ea675d4 Merge branch 'bugfix/244'
|\
| * cead734 (bugfix/244) Rename zoo1[34]C to zoo1[34]Ctot
|/
*   21f77a2 (tag: marbl0.28.5, origin/master) Merge remote-tracking branch 'klindsay/bugfix/253'
|\
| * b986d61 (klindsay/bugfix/253) updated default_settings values from jkmoore
|/
* 5ce7fe9 (tag: marbl0.28.4) Bugfix: indices of diag%field_3d were wrong

• Besides a cleaner graph, I think rebasing provides a better conflict-resolution method. Hypothetically, suppose there had been a conflict in one of Keith's enhancment/biomass_lim_factors commits (0ff947d correct units on some diagnostics, to have a concrete example). In merging, the conflict would have been resolved in 3633890 (HEAD -> master) Merge branch 'enhancment/biomass_lim_factors' but rebasing would have forced us to resolve the conflict in 1447c0a correct units on some diagnostics
• I can think of two downsides to rebasing, though in my mind the positive outweighs the negative:
  1. Rebasing changes the git history, which means the local repository will be out-of-sync with the remote (our workflow assumes the remote is a personal fork of the main MARBL repository, and what I do on my fork should not affect other developers)
  2. With rebasing, it's no longer clear what version of MARBL the development originally started on and it is impossible to reconstruct sandboxes originally used for testing
• Another option: Bill Sacks recommends NOT using git rebase, and points out that git log --first-parent can be used instead of the graph.

MERGING --first-parent

* a732fc0 (HEAD -> master) Merge branch 'enhancment/biomass_lim_factors'
* 7ba7e24 Merge branch 'bugfix/248'
* 5581342 Merge branch 'bugfix/255'
* 9effcfb Merge branch 'bugfix/244'
* 21f77a2 (tag: marbl0.28.5, origin/master, origin/HEAD) Merge remote-tracking branch 'klindsay/bugfix/253'
* 5ce7fe9 (tag: marbl0.28.4) Bugfix: indices of diag%field_3d were wrong

This would require putting more information in the merge commit log instead of the default Merge branch ..., e.g. the summary from a pull request.