-
Notifications
You must be signed in to change notification settings - Fork 15
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
Remove climo_land_subsection #679
Conversation
@chengzhuzhang I think I can just merge this. Per the discussion at #669 (comment). |
@forsyth2 please follow through code review before merging. I have some questions for this one. |
@chengzhuzhang Sorry for the hasty merge. I did not think it required further code review; we had thoroughly discussed the design decision and the removal was simple. Please let me know if I should revert the commit. What are your questions? |
# Note: often `lat_lon_land` will require a different climo_subsection | ||
# than the other sets (e.g., a climo subsection that includes land). | ||
# That means this set will often need to be run as a separate subtask. | ||
"lat_lon_land", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. I was unsure about this line, and comment. But reading more of the code, I understand how dependencies handles differently for land vs atm, via "climate_section".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My configuration file is working now. I can confirm #669 is resolved.
Great, thank you @chengzhuzhang! |
Summary
Objectives:
climo_land_subsection
, opting to use the existingclimo_subsection
instead.Issue resolution:
Select one: This pull request is...
Small Change