-
Notifications
You must be signed in to change notification settings - Fork 1
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
Compiler in base-spack #62
Conversation
… in later Dockerfiles/scripts
67e5126
to
f3b1bb8
Compare
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 have some comments!
I can't say I have checked it exhaustively, but I can do another pass if required.
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.
All good. Though just a cursory run through before I have to head off.
Only point in relation to something Aidan mentioned (and this my opinion only) is to use traditional GNU stuff (sed/awk/etc). Jq is great and I've used it often - just that it is 3rd party. Again just my opinion, I'm an old timer (pre-JSON standard) so probably a bit conservative in what I use for parsing :) Otherwise happy to use new stuff - just depends if it is critical or not. Depending on definition of citical!
@Quartzer2 thats a fair opinion too! Looking through the |
Indeed it is. But as you say @CodeGat we're already using it and it is very nice and fast and actually parses the JSON, so has more chance of picking up problems, and working with a wider range of possible inputs. I like jq because it is obvious what it is doing. We could arguably make it even more obvious using the full command line option I admit I couldn't think of a way the current method breaks, but I only spent 5 minutes thinking, and I may not be very imaginative. I did think of a way: DOS line-endings, but that seems a bit far-fetched, so I'm not counting it. |
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 great!
Thanks for the good work.
…ency-image pipeline, using jq
ec24506
to
8735953
Compare
Pushing the compiler down into base-spack as it is not updated as frequently in the pipeline and can therefore help compilation times. This PR changes the workflows somewhat, in which the matrix is created relatively early in the pipeline, and is fed into the new workflow (dependency-image-pipeline.yml)
Should close #50 !