-
Notifications
You must be signed in to change notification settings - Fork 14
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 unused cmake macros #40
Conversation
@aerorahul @DusanJovic-NOAA @kgerheiser @mark-a-potts I simply removed everything except the NetCDF, ESMF and WGRIB2 find macros. If you think or already know that some of the stuff I deleted is required, please let me know. |
dang! |
@aerorahul this works for me in NCEPLIBS-external (see PR NOAA-EMC/NCEPLIBS-external#57). Can you or somebody else please test that this trimmed down version still works with the NCEPLIBS-xyz that use CMakeModules (and with any other model that might use CMakeModules that I am not aware of)? |
@climbfuji |
That would be great, thanks. I noticed that a few of the NCEPLIBS-xyz are still using (or at least having) CMakeModules in .gitmodules.
… On Jul 29, 2020, at 1:47 PM, Rahul Mahajan ***@***.***> wrote:
@climbfuji <https://github.com/climbfuji>
I can test with NCEPLIBS and UFS-utils.
I don't know of any other application using CMakeModules.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#40 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5C2RK4XMLLTSBJXYEHLFDR6B4FHANCNFSM4PK5AVDA>.
|
wgrib2 is in NCEPLIBS now, along with its CMake build and package config. |
Does this mean I should remove the FindWGRIB2.cmake from the repository as well? |
I'm not sure. It was just merged today and hasn't been installed anywhere yet and there may be some other applications (UFS_UTILS, maybe Post?) that need to be converted as well. Not that any of that is difficult, but it's not ready at this exact moment. |
Thanks, I'll leave it as-is for now.
… On Jul 29, 2020, at 1:54 PM, Kyle Gerheiser ***@***.***> wrote:
I'm not sure. It was just merged today and hasn't been installed anywhere yet and there may be some other applications (UFS_UTILS, maybe Post?) that need to be converted as well. Not that any of that is difficult, but it's not ready at this exact moment.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#40 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB5C2ROR5M7HT3E2W5IFQH3R6B47NANCNFSM4PK5AVDA>.
|
@climbfuji |
Thanks, @aerorahul. Once we know that ufs_utils works as well, we can merge this PR. Are you going to update the submodule pointers in those repositories you tested? |
Sure @climbfuji. I can update the submodule pointers in those repos. I want to make sure everything works before I issue PR's. Would that be OK? |
Absolutely, this can wait. Thanks for testing all the codes. |
@climbfuji |
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.
works with nceplibs-xyz and ufs_utils.
Great, thanks. So, can we merge? |
I think so. |
Heavy-axe cleanup: remove everything that isn't used right now.
Testing ongoing with NCEPLIBS-external, develop branch. Also need to test with the various NCEPLIBS.