NorESMhub / BLOM

Bergen Layered Ocean Model
GNU Lesser General Public License v3.0
16 stars 25 forks source link

forget to modify buildcpp in Isotopoes write/read #278

Closed monsieuralok closed 8 months ago

JorgSchwinger commented 8 months ago

Hi Alok,

in this PR, if I understand correctly, you only want to modify the buildcpp script (remove the error message if C-isotopes are activated with active sediment)? However, why is then the file mo_apply_rivin.F90 modified? It looks like these are the same changes that have been already pulled into master? Also, the line in buildcpp should be removed, not commented out. And a descriptive title for the PR would be nice, actually saying what it is doing, e.g. "Change buildcpp to allow to active C-isotopes with active sediment".

I see that you are issuing this PR from the master branch of your fork. It is much easier if you (starting from a master that is up-to-date with the current upstream-master) create a branch where you implement the changes you want to bring into upstream-master, push this branch to your fork of BLOM, and create the pull request. After the pull request is merged into upstream-master you can easily update the master of your fork and local copies (which is difficult if you do the PR from the master branch of your fork).

Unless I have misunderstood something, I would suggest to delete this PR, and bring in a new, clean PR only removing the one line in buildcpp. I can offer to do this, since we would like to have this in quickly for the creation of the final CMIP6 backwards compatible release of BLOM/iHAMOCC, just let me know.

monsieuralok commented 8 months ago

@JorgSchwinger it would be nice if you can do it as it is very minor change for buildcpp It is true that I only wanted to modify buildcpp

JorgSchwinger commented 8 months ago

Ok, there are anyway some other changes to buildcpp upcoming - we will include this.