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

Place dependencies into separate file #2838

Merged
merged 7 commits into from
Jan 18, 2024

Conversation

K20shores
Copy link
Contributor

@K20shores K20shores commented Jan 10, 2024

This separate outs the dependencies and partially addresses #2713

  • All find_package calls are now in the dependencies.cmake file, making it easyish to see in one place what is needed
  • Added a file to start to collect the macros and functions as recommended by the linked issue

@ZedThree
Copy link
Contributor

Probably quite a lot of this could also be moved into separate Find*.cmake files to tidy up even further

@K20shores K20shores marked this pull request as ready for review January 16, 2024 21:48
@K20shores
Copy link
Contributor Author

@WardF after @ZedThree's suggestion, the tests pass and it appears this PR is ready to go.

@WardF
Copy link
Member

WardF commented Jan 17, 2024

Thanks @K20shores and @ZedThree. Hope everybody stayed/is staying warm. Now that things are thawing out in rural Oregon, I should be able to resume review of the PR backlog.

@WardF WardF self-assigned this Jan 17, 2024
@WardF
Copy link
Member

WardF commented Jan 17, 2024

One last change occurs to me; the new file needs to be added to theEXTRA_DIST definition in the makefile.am file in the corresponding directory. This way, when we create tarballs with make dist (using an autotools-based build), the tarball will have all the files needed for cmake-based builds.

@WardF
Copy link
Member

WardF commented Jan 17, 2024

I can make that change; other than that, this looks great, thanks!

@K20shores
Copy link
Contributor Author

@WardF Makefile.am seems to include the cmake directory; that's where the dependencies.cmake and netcdf_functions_macros.cmake file are. Does it still need to be added to Makefile.am?

@WardF
Copy link
Member

WardF commented Jan 17, 2024

If it includes the directory we should be good. I'll double check to be sure, but that's probably got it. Thanks!

@WardF WardF merged commit 74141e7 into Unidata:main Jan 18, 2024
99 checks passed
@K20shores K20shores deleted the modernize_reorganize branch March 22, 2024 18:04
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.

3 participants