-
Notifications
You must be signed in to change notification settings - Fork 21
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
Update tracking CI, add new CI action for LTO and modify other actions to use just #1462
Conversation
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.
This is great :) definitely a step in the right direction.
I don't really have any notes except a question. Do you think it would be helpful to summarize the default ci building in a just recipe to avoid copying it around?
[private]
ci-build: install-denv init (configure-force-error) build
(The parentheses help inform just
that we aren't giving any more arguments to the configure step which would normally swallow all following arguments as build configuration.)
Then the CI calls would be
run: just ci-build
might be helpful, might not be.
Hmm, I dont know, I see your point that this would certainly be much more compact, but I kinda like that it's spelled out, just for making it more obvious. And also it's more segmented right now in case some of the prev steps failed. So I'm kinda leaning forward keeping it as is. |
ea02242
to
9d15166
Compare
Compilation part looks good! Running the pn validation here |
Yaay, ecalPN is back to work! |
47ba751
to
9abdcb9
Compare
I am updating ldmx-sw, here are the details.
What are the issues that this addresses?
Resolves #1459
Check List
Well the test of the pudding is to see what github does with these actions: