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

USE_LOGGING not propagated to LOGGING preprocessor macro #44

Merged
merged 1 commit into from
Feb 26, 2016
Merged

USE_LOGGING not propagated to LOGGING preprocessor macro #44

merged 1 commit into from
Feb 26, 2016

Conversation

nncarlson
Copy link
Contributor

If USE_LOGGING=ON this is not propagated to a -DLOGGING compile line definition, which always results in an empty nf_logging.F90 file. (The NAG compiler refuses to accept an empty file.) This commit addresses this by simply removing the #ifdef from nf_logging.F90; it seems to be unnecessary because both the cmake and autoconf configurations seem to be very careful about only including this source file when USE_LOGGING is ON.

@WardF
Copy link
Member

WardF commented Feb 26, 2016

Thanks for the contribution, it is very much appreciated! I'll review this after the automated checks complete and merge for the upcoming release!

@nncarlson
Copy link
Contributor Author

My first time seeing Travis-CI in action -- pretty slick! (albeit a little slow)

@WardF
Copy link
Member

WardF commented Feb 26, 2016

It is, and I highly recommend it if at all possible. It also can run much faster, but we're using docker for our test matrices (details here if you're interested) and that slows the process down a bit as well.

@WardF
Copy link
Member

WardF commented Feb 26, 2016

Ok, reverting this change after a fashion; it turns out the previous logic was correct. USE_LOGGING is set if nc_set_log_level is found in the netCDF C library. If it's not, then logging is not available.

I'm adding a message to the configuration summary to make it more clear whether logging is available or not.

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

Successfully merging this pull request may close these issues.

2 participants