-
Notifications
You must be signed in to change notification settings - Fork 106
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(workflows): consolidate workflows based on their purpose #7616
Conversation
Error: ``` Validation Failed: {"resource":"Release","code":"invalid","field":"target_commitish"} ``` Fixes: release-drafter/release-drafter#1125
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: teor <teor@riseup.net> Co-authored-by: Arya <aryasolhi@gmail.com>
EDIT: Actually, let's keep the test coverage as is, I'll make the workflow segregation in another PR fix: revert test coverage on CD
If we don't want to release a production image with issues, I think running this in the A more "elegant" solution is also making the push on |
All recommendations were applied |
I made a last fix to the docker config test to accept different images. Even though this was (mainly) required in this spots: fd6aaf2#diff-d43d0df5f20e7f637ae74715b01cc8a0b644574a546a5242c78b9b8d4bab9571R217 I added it to other places for consistency |
If this test fails https://github.com/ZcashFoundation/zebra/actions/runs/6553940936/job/17800228563?pr=7616 after fix(dockerfile): use variables or default for config path and file it might because this file does not exists in the release image: If that's the case, we can remove this test from CD, or use a file generated by Zebra, different than the one created by the entrypoint |
I'm going to delete the old branch protection rules now, so this PR and other PRs can merge. Admin: This PR needs 4 new branch protection rules after it merges:
I'll go update the patch jobs to match in this PR now. |
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.
Thank you for all your work on this, I think we're good to go here
We like to stop these kinds of bugs before we merge a PR, because fixing them after we merge or tag the release takes extra time. This is particularly important if we're trying to get a release out quickly to meet a deadline or fix a security issue. It's ok to also test in the
That seems like a great idea! There's no need to have the release depend on the release image tests, as long as we're sure the tested release image is always the same as the DockerHub push released image. (If we're testing every PR, it's unlikely that the test will fail during the release itself.) |
Motivation
Some workflows have different requirements like resources, time and dependencies, and even have jobs with different objectives (in some cases, not related to other workflow jobs).
Fixes: #6166
Fixes: #6167
Depends-On: #7660
Specifications
Complex Code or Requirements
To comply with the naming convention, some jobs had to be moved from one workflow to another, and we have a completely new workflow, but no new jobs, which consolidates integration tests (sync tests) which required to be deployed in GCP
Solution
Review
Reviewer Checklist
Follow Up Work