-
Notifications
You must be signed in to change notification settings - Fork 50
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
Streamline the checks for missing installations #795
Streamline the checks for missing installations #795
Conversation
Instance
|
Instance
|
Instance
|
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.
Looks fine mostly. A few suggestions for the script only_latest_easystacks.sh
. Might also be worthwhile to put curly brackets around all variable names.
The one rebuild easystack file that was moved, it was not necessary for this PR, just something you noted?
@@ -0,0 +1,42 @@ | |||
#!/bin/bash | |||
EESSI_VERSION=${EESSI_VERSION:-"2023.06"} |
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.
If you source init/eessi_defaults
you don't have to change the version here later.
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.
I wanted this to be configurable in CI via setting the envvar
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.
The value here is actually set in the CI configuration, this is only here as a fall back in case someone explicitly calls this script for some reason
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.
Benefit of sourcing init/eessi_defaults
is that one has to change the default in a single place only. In CI, one can set EESSI_VERSION_OVERRIDE
before sourcing the defaults script.
Anyhow, I'll approve the PR.
@trz42 One rebuild easystack was moved because it was in the wrong location, it should have been under the |
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.
Looks good.
PR merged! Moved |
PR merged! Moved |
1 similar comment
PR merged! Moved |
This only checks the easystack files that are using the latest releases of EasyBuild (since this is our policy and there is little point in checking older easystacks over and over)