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.

Sign up for GitHub

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

Jump to bottom

#19: Update spack to look for magistrate #20

Merged
merged 2 commits into from
Jul 16, 2024
Merged

#19: Update spack to look for magistrate #20

merged 2 commits into from
Jul 16, 2024

Conversation

JacobDomagala
Copy link
Contributor

@JacobDomagala JacobDomagala commented Jul 10, 2024 •
edited
Loading

Fixes #19 Fixes #18

@JacobDomagala JacobDomagala self-assigned this Jul 10, 2024
@JacobDomagala JacobDomagala linked an issue Jul 10, 2024 that may be closed by this pull request
Update spack to look for magistrate #19
Closed
@JacobDomagala JacobDomagala requested a review from nmm0 July 10, 2024 10:39
nmm0
nmm0 approved these changes Jul 10, 2024
View reviewed changes
Copy link
Contributor

@nmm0 nmm0 left a comment

Choose a reason for hiding this comment

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

Overall I think this looks good, but we may want backwards compatibility with older Darma versions? I suppose it depends on if anyone is actually using these packages

@cz4rs cz4rs self-requested a review July 11, 2024 18:20
cz4rs
cz4rs approved these changes Jul 11, 2024
View reviewed changes
Copy link

@cz4rs cz4rs left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@nmm0 Honestly I'd be tempted to just merge this as is and see if anyone complains.

@JacobDomagala JacobDomagala marked this pull request as draft July 11, 2024 19:15
@JacobDomagala
Copy link
Contributor Author

Moving this to draft. Tested it locally and it still fails

@JacobDomagala JacobDomagala force-pushed the 19-update-spack-to-look-for-magistrate branch 8 times, most recently from 4ade027 to 7f4fe93 Compare July 11, 2024 21:26
@JacobDomagala JacobDomagala force-pushed the 19-update-spack-to-look-for-magistrate branch from 7f4fe93 to e57e8ac Compare July 11, 2024 21:27
@JacobDomagala JacobDomagala marked this pull request as ready for review July 11, 2024 21:33
@JacobDomagala
Copy link
Contributor Author

Ok, works now. I also added CI that runs spack

@nmm0 nmm0 mentioned this pull request Jul 16, 2024
Open
@lifflander lifflander merged commit b167719 into master Jul 16, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lifflander lifflander lifflander approved these changes

@cz4rs cz4rs cz4rs approved these changes

@nmm0 nmm0 nmm0 approved these changes

Assignees

@JacobDomagala JacobDomagala

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update spack to look for magistrate Test spack install
4 participants
@JacobDomagala @lifflander @cz4rs @nmm0