Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

forget to modify buildcpp in Isotopoes write/read #278

Closed
wants to merge 3 commits into from
Closed

forget to modify buildcpp in Isotopoes write/read #278

wants to merge 3 commits into from

Conversation

monsieuralok
Copy link
Collaborator

No description provided.

@JorgSchwinger
Copy link
Contributor

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
Copy link
Collaborator Author

@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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants