Skip to content

Update namespace and targets in packaging#83

Merged
boulderdaze merged 3 commits intomainfrom
small_change_in_packaging
Apr 10, 2025
Merged

Update namespace and targets in packaging#83
boulderdaze merged 3 commits intomainfrom
small_change_in_packaging

Conversation

@boulderdaze
Copy link
Copy Markdown
Contributor

Updated namespace and installation target script in packaging

@boulderdaze boulderdaze self-assigned this Apr 8, 2025
@boulderdaze boulderdaze requested a review from K20shores April 8, 2025 23:27
Comment thread packaging/CMakeLists.txt Outdated
DESTINATION
${cmake_config_install_location}
NAMESPACE open_atmos::
NAMESPACE mechanism_configuration::
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
NAMESPACE mechanism_configuration::
NAMESPACE musica::

In micm and tuvx, we use musica for the namespace. Perhaps we should do the same here

We will also need to change it in the file that defines the cmake library

https://github.com/NCAR/MechanismConfiguration/blob/main/src/CMakeLists.txt#L6

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, that's a good point. Updated.
How about the prefix of the CMake options?

option(OPEN_ATMOS_ENABLE_TESTS "Build the tests" ON)
option(OPEN_ATMOS_ENABLE_PYTHON_LIBRARY "Build the python library" ON)
option(OPEN_ATMOS_ENABLE_PIC "Build the library with position independent code" ON)

https://github.com/NCAR/MechanismConfiguration/blob/main/CMakeLists.txt#L24-L26

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, we may as well go ahead and change those. Typically we prefix those with the name of the reposiry (musica uses MUSICA, micm uses MICM). I gess MECH_CONFIG would be a good prefix? I'm open to whatever you think is good

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I like that. Yeah let me include them in this PR.

@boulderdaze boulderdaze requested a review from K20shores April 9, 2025 16:12
@boulderdaze boulderdaze merged commit 6fde78f into main Apr 10, 2025
33 checks passed
@boulderdaze boulderdaze deleted the small_change_in_packaging branch April 10, 2025 15:58
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.

2 participants