Merge Spin 1951 to BLAZE_9184

[AlisonBennett]

CABLE

Thank you for submitting a pull request to the CABLE Project.

Description

Merge of minor differences in the Spin_1951_draft_ACB branch with the BLAZE_9184 branch in preparation for merge of BLAZE_9184 into NESP2pt9_BLAZE.

Type of change

Branch Merge.

Please add a reviewer when ready for review.

[AlisonBennett]

@ccarouge @SeanBryan51 read the docs has failed on our merge. Can you please advise? Thanks!

[ccarouge]

@AlisonBennett The documentation isn't implemented on the legacy branches, it's ok to let it fail. The only time it matters is PRs going into the main branch.

edit: okay - they're there. The diffs are between BLAZE_9184 and the Spin_1951 branches - and we'd implemented these in BLAZE_9184 already.

[har917]

@AlisonBennett Where have all the other changes related to %potstemnpp gone?

[AlisonBennett]

@har917 I think we reviewed the other changes for PotStemNPP when we resolved the merge conflicts, so they are not available in the merge. But maybe something to check with @ccarouge

[ccarouge]

I was puzzled by some of the comments in the review, so I went and checked the branch graph. Why are you merging Spin_1951_draft_ACB into BLAZE_9184 when the Spin branch stems out of the NESP_2pt9_BLAZE branch? Why not merge it into NESP_2pt9_BLAZE and then merge BLAZE_9184 and NESP_2pt9_BLAZE together? It would prevent some of the weird result about what has changed or not and where does it come from.

[ccarouge]

For the PotStemNPP changes, you are correct Alison that all the code parts where merge conflicts happened are now identical between the 2 branches so they will not appear in this PR.

Edit: re-reading the history, this comment comes late as you have already done some work for the merge with the conflicts. But it would be good to know your reasoning because I think the way I propose here would give the same result with less headache. In general, you want to merge a branch with the branch it stems from. It is very rare to merge to another branch. Especially in this case, when BLASE_9184 will be merged into NESP_2pt9_BLAZE if I remember correctly.

[AlisonBennett]

Thanks for looking at what we are doing. I think our reasoning was that because @har917 and I had separately implemented some of the same fixes in our branches, we wanted to check that we had both done the same thing before merging into the NESP2pt9_BLAZE branch. It turns out our implementations were slightly different and that I had made an error, so the merge allowed us to resolve those differences before merging to NESP2pt9_BLAZE.

[har917]

@ccarouge Are you happy that we go ahead and merge these two branches- or should we discard then merge spin_1951 into NESP2pt9, update BLAZE_9184, test, then merge back into NESP2pt9_BLAZE?

Also - which merge option should we be using?

[AlisonBennett]

Yes we can go ahead and merge the branches now. I think we go ahead with the original plan.
Spin_1951 -> BLAZE_9184 Then, BLAZE_9184 -> NESP2pt9_BLAZE

[ccarouge]

@AlisonBennett Don't change the plan now. My comment was more for general knowledge. For the merge option, we are using the Merge pull request option. It's the safest one when people are git beginners.