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

Silence error messages in getChannel() and getWavelength(), and add header file in process library #249

Merged
merged 4 commits into from
Mar 7, 2024

Conversation

rmontuoro
Copy link
Contributor

Resubmitting PR #248 against develop to address issue #247.

@tclune
Copy link
Contributor

tclune commented Aug 4, 2023

@amdasilva This PR exposes one of the downsides of having the process library not depending on MAPL. We lose some useful error handling utilities this way. At what point to we accept that CCPP is no longer something that needs support?

@bbakernoaa
Copy link

bbakernoaa commented Aug 4, 2023 via email

@tclune
Copy link
Contributor

tclune commented Aug 4, 2023

@bbakernoaa Are you aware of any activity in that direction? I'm sure we're not changing anything without lots of review to be certain we are not closing any important doors.

@bbakernoaa
Copy link

bbakernoaa commented Aug 4, 2023 via email

@tclune
Copy link
Contributor

tclune commented Aug 4, 2023

@rmontuoro Unfortunately, it is not just the macros. There are a handful of procedures involved as well. We could create a "MAPL-lite" and make that a dependency. If we get frustrated enough, I suppose we could look into that.

I'd also like to use pflogger in that layer, but same issue ...

@mathomp4 mathomp4 added the 0 diff The changes in this pull request have verified to be zero-diff with the target branch. label Sep 25, 2023
vbuchard
vbuchard previously approved these changes Mar 7, 2024
Copy link
Contributor

@vbuchard vbuchard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok.

Copy link
Collaborator

@amdasilva amdasilva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@vbuchard vbuchard merged commit 912fc95 into GEOS-ESM:develop Mar 7, 2024
4 checks passed
@amdasilva
Copy link
Collaborator

@amdasilva This PR exposes one of the downsides of having the process library not depending on MAPL. We lose some useful error handling utilities this way. At what point to we accept that CCPP is no longer something that needs support?

@tclune No, it is a bad idea to have any dependency on MAPL and ESMF. This defeats the purpose of isolating the ESMF code. Also, any simple offline utility that needs any of this functionally will need to depend on a gigantic library. Keep the Process Library self contained and basic.

@tclune
Copy link
Contributor

tclune commented Mar 7, 2024

I understand. Just illuminating the tradeoff. Dependencies are also bad. I hate shades of grey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0 diff The changes in this pull request have verified to be zero-diff with the target branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants