-
Notifications
You must be signed in to change notification settings - Fork 7
Fix syntax for esm1.5 (doesn't use cable as a library) #372
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
base: main
Are you sure you want to change the base?
Conversation
|
Are these syntax changes for Spack 1.0? |
|
Nope unrelated - the old syntax was wrong |
|
Fair enough, approved |
harshula
left a comment
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.
Hi @anton-seaice , What's the reason for the file .github/build-ci/manifests/um7/intel.spack.yaml.j2 being renamed to .github/build-ci/manifests/access-esm1p5/intel.spack.yaml.j2?
|
git interpreted the changes slightly strangely. I added the file I duplicated the file
So that there is coverage of both builds |
|
Hi @anton-seaice , Any reason you didn't do something like: https://github.com/ACCESS-NRI/access-spack-packages/blob/main/.github/build-ci/manifests/oasis3-mct/intel.spack.yaml.j2 ? |
|
No that work would fine as well, I think it's easier generally to debug failures which are more granular |
|
Hi @anton-seaice , There's also additional overhead when additional environments need to be setup. You make a good point regarding debugging. I'll discuss that with @CodeGat when he gets back. For the time being, can you please add both specs to the existing manifest? e.g. https://github.com/ACCESS-NRI/access-spack-packages/blob/main/.github/build-ci/manifests/oasis3-mct/intel.spack.yaml.j2 . It will also make the transition to APIv2 easier to track and verify. |
|
I think i've made the suggested change @harshula |
harshula
left a comment
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.
Thanks! Please remember to Squash merge
|
Is merge commit ok? Just two commits with different scope in each |
|
Hi @anton-seaice , Yes, sure. That's probably an indication that some of the changes in this PR, should be in a separate PR. |
Closes #371
This fixes a syntax error in the um7 package.
This also adds ci builds for access-esm1.5