Skip to content

Spack Components for OM3#171

Merged
anton-seaice merged 73 commits intomainfrom
163-om3-components
Apr 7, 2025
Merged

Spack Components for OM3#171
anton-seaice merged 73 commits intomainfrom
163-om3-components

Conversation

@anton-seaice
Copy link
Copy Markdown
Contributor

@anton-seaice anton-seaice commented Nov 27, 2024

This is an extremely draft collection of changes to spack-packages to build OM3 by component.

I've been building access3-share with '+install_libraries' in the spec, I haven't looked at building with an executable yet

I've been using these git versions :

https://github.com/ACCESS-NRI/CICE/tree/5-port_cmake_build

and

https://github.com/ACCESS-NRI/access3-share/tree/share_prototype

Example deployment: ACCESS-NRI/ACCESS-OM3#35

@anton-seaice anton-seaice marked this pull request as draft November 29, 2024 04:29
@anton-seaice
Copy link
Copy Markdown
Contributor Author

Contributes to ACCESS-NRI/access-om3-configs#346

@anton-seaice
Copy link
Copy Markdown
Contributor Author

This is still a draft but is ready for a review for style / approach.

I've only implemented for CICE6 at this point, see the example deployment in ACCESS-NRI/ACCESS-OM3#35

Copy link
Copy Markdown
Member

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

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

I have pending comments, so had to submit a review for some reason.

@aidanheerdegen aidanheerdegen mentioned this pull request Apr 2, 2025
Copy link
Copy Markdown
Contributor

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

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

Fixes for failing linting checks

dougiesquire
dougiesquire previously approved these changes Apr 3, 2025
Copy link
Copy Markdown
Contributor

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

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

This looks good from my perspective, thanks @anton-seaice. My preference is to wait for approval from @aidanheerdegen before merging, particularly in regards to the setting of versions.

@anton-seaice
Copy link
Copy Markdown
Contributor Author

Last call for reviews @aidanheerdegen and @harshula :)

Copy link
Copy Markdown
Collaborator

@harshula harshula left a comment

Choose a reason for hiding this comment

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

@anton-seaice
Copy link
Copy Markdown
Contributor Author

I made some changes for these - let me know if I missed anything

Copy link
Copy Markdown
Collaborator

@harshula harshula left a comment

Choose a reason for hiding this comment

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

Well done!
Please remember to squash the commits and spend a bit of time on creating a concise but meaningful commit message.

Copy link
Copy Markdown
Member

@aidanheerdegen aidanheerdegen left a comment

Choose a reason for hiding this comment

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

One question, otherwise happy for you to merge.

One point for a possible future change is to keep all the common depends_on logic in a single place and include them into the packages. I guess a mixin class?

Copy link
Copy Markdown
Contributor

@dougiesquire dougiesquire left a comment

Choose a reason for hiding this comment

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

I resolved some review comments that I started, but there are still a few others started by others that I didn't feel qualified to resolve

@anton-seaice anton-seaice merged commit 05e6ce7 into main Apr 7, 2025
1 check passed
@anton-seaice anton-seaice deleted the 163-om3-components branch April 7, 2025 00:52
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.

6 participants