-
Notifications
You must be signed in to change notification settings - Fork 287
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
refactor(ci/github): run weaver tests on schedule, re-use workflows #3437
refactor(ci/github): run weaver tests on schedule, re-use workflows #3437
Conversation
fc746d5
to
2fd322f
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.
LGTM, just one optional change if you want to, i.e. rename Test Weaver
to Weaver_CI
, just like Cactus_CI
just to be consistent in naming.
But this is optional just aesthetic, I don't have much issue with current one also.
1a81c91
to
ae5d5ea
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.
@jenniferlianne Agreed, the actionlint looks bugged out. Could you please try to upgrade the actionlint version to the current latest instead of whatever we are using at the moment? I'm hoping that this bug was discovered and fixed a while back!
Also plus one for the rename that @sandeepnRES mentioned above.
I've removed all weaver checks from required ones from branch protection rules (an exception for this PR as it is refactoring the CI itself), as it was blocking the PR to be merged. |
df6b2a6
to
c3c2895
Compare
@petermetz I have updated the ActionLint version to the latest, but it still fails. The linter might be a getting stuck on a permissions issue between repos. (Settings->General->Actions Permissions). Mine is set to run all. |
@jenniferlianne Now I remember! We didn't have time to fix all the linter errors in certain yaml files at the introduction of the ActionLint job and there was no documented way to ignore them so the ActionLint job deletes a list of the yaml files that we wanted to ignore as a workaround. I've just pushed a potential fix, but let's see if it actually works. |
ea0a0a9
to
a1834cb
Compare
@jenniferlianne I've put the fix here: #3446 once that gets merged to main and this one rebased onto main then the ActionLint job will start passing on this PR as well. |
Primary change: Creates a central workflow to run all 13 weaver test workflows from either a push, a pull request, a github command, or a schedule. Passes a 'run_all' boolean to sub-workflows to specify if all tests should be run, not just those with changes. Secondary changes: - update concurrency groups to allow triggering from central workflow - remove ability to run the 13 workflows individually to declutter ci menu check whether RUN_ALL can be calculated in the environment instead of as a job rename weaver test file update actionlint to latest Depends on hyperledger-cacti#3446 Signed-off-by: Jennifer Bell <jenniferlianne@gmail.com>
a1834cb
to
e8763fd
Compare
This PR/issue depends on:
|
Primary change:
Creates a central workflow to run all 13 weaver test workflows from either a
push, a pull request, a github command, or a schedule.
Passes a 'run_all' boolean to sub-workflows to specify if all tests should be run, not
just those with changes.
Secondary changes:
Notes:
Depends on #3446
Pull Request Requirements
upstream/main
branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.-s
flag when usinggit commit
command. You may refer to this link for more information.Character Limit
A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.