-
Notifications
You must be signed in to change notification settings - Fork 108
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
ref(workflow): move most scripts to their own executables #8005
Conversation
G-cloud issues, i am updating the branch to run everything again and check if that was temporal or already resolved. |
Unfortunately this seems to be a bug in the refactor. https://github.com/ZcashFoundation/zebra/actions/runs/7023238162/job/19109744164?pr=8005 |
Some fixes were applied. This should be good now |
https://github.com/ZcashFoundation/zebra/actions/runs/7094975945/job/19311396467 Updating branch to re run the CI. |
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 think this PR simplifies the CI by a lot so i will like to see it merged. However, it seems there is a problem in the changes.
The CI is stuck here:
https://github.com/ZcashFoundation/zebra/actions/runs/7101877538/job/19331501819?pr=8005
What i am most afraid of if that it seems to not be picking the right disks. @gustavovalverde can you take a look and clarify ?
This seems to be the same bug with the runners, I can do the same temporary fix if Gustavo isn't around. |
This seems to be maybe finding the incorrect GCP resource? |
That's correct. But it's actually because a wrong assignment was being made, and previously the disk was not matching correctly, so @oxarbitrage was right being afraid with CI failing. There's just a missing fix which I'll end up doing tomorrow. |
@oxarbitrage this is finally good! The PR is failing as codespell is failing to build, but that's unrelated: https://github.com/ZcashFoundation/zebra/actions/runs/7143157187/job/19454023893?pr=8005 |
I've scheduled this after the scanner tests merge, because they are blocking other work. |
Co-authored-by: Marek <[email protected]> Co-authored-by: teor <[email protected]>
)" This reverts commit d85b010.
Motivation
Some CI files are hard/understand as they have bash scripts included, some of this scripts are also hard to maintain or lint considering they're inside a
.yml
and not in an actual.sh
Some of these scripts might also be useful as a separate file to run independently for specific use cases, like searching for disks in GCP (but that's not part of the scope of this PR)/
Fixes #6168
Depends-On: #8083
PR Author Checklist
Check before marking the PR as ready for review:
For significant changes:
If a checkbox isn't relevant to the PR, mark it as done.
Complex Code or Requirements
Some scripts required refactoring for consistency and better use outside the workflow YAML
Solution
./github/workflows/scripts
directoryReview
We need to confirm the right disk are being pulled, comparing with recent PRs
Reviewer Checklist
Check before approving the PR:
PR blockers can be dealt with in new tickets or PRs.
And check the PR Author checklist is complete.
Follow Up Work
Make this scripts usable as a standalone script