more CI improvements #33
Conversation
Luap99
force-pushed
the
validate
branch
5 times, most recently
from
98dc475 to
49a3c1c
Compare
Merge the per repo codespell configs into one file and then just have the one github action job which runs codespell. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Update to latest version. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
It seems easier to run git-validation as part of github actions as we can get the proper PR commit count there. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Signed-off-by: Paul Holzinger <pholzing@redhat.com>
It validates all modules now. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
We do the validation in github action now which should be faster and more consitent with each module. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
Looks like we have a (new?) flake in CI where the db is not ready. In such case dump the container logs and show podman ps output so we see what is going on when it happens again. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
By default the action tries to cache the modules which is good but it doesn't handle "monorepos" by default, we need to provide the paths to the go.sum files. see https://github.com/actions/setup-go?tab=readme-ov-file#caching-dependency-files-and-build-outputs Signed-off-by: Paul Holzinger <pholzing@redhat.com>
We don't have to run the cross builds as part of each storage driver test, in fact the main cirrus.yml already has a separate cross build task which already covers this so we can simplify and thus speed up the storage test task. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
As it seems the DB is still starting up after 20s so give it more time, increase timeout to 40s. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
No task should take longer than that, having a big timeout just means if a test/task hangs it waits for 2h for no reason. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
|
LGTM |
See containers#280, the custom file regex manager now uses the managerFilePatterns key which accept an array. Note the .github/workflows/validate.yml doesn't actually exists right now but is again being renamed in containers#33 so let's just keep it at that. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
See containers#280, the custom file regex manager now uses the managerFilePatterns key which accept an array. Note the .github/workflows/validate.yml doesn't actually exists right now but is again being renamed in containers#33 so let's just keep it at that. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
|
LGTM |
| - name: install deps | ||
| run: go install github.com/vbatts/git-validation@v1.2.2 | ||
| - name: run git-validation | ||
| run: git-validation -q -run DCO,short-subject,dangling-whitespace -range "HEAD~${{ github.event.pull_request.commits }}..HEAD" |
There was a problem hiding this comment.
Low-priority: Does this correctly handle merge requests and the like?
I think conceptually we should include all commits not already present on the target branch, and it’s not obvious to me that counting commits is an accurate way to achieve that.
There was a problem hiding this comment.
Why would counting not work? This should return the number of commits in the PR (well what github reports) and then we lint the diff between that commit in HEAD which should aalways check all commits in any given PR.
Do you mean like merge commits within a PR? They should still be reported and checked as normal commit I think. I mean feel free to tests the scenario you can think of out.
There was a problem hiding this comment.
Yes, I was thinking something like containers/image#2876 which goes
- o - ……… - o = upstream/main
\ - old -\ - newer
i.e the branch is “rebased on top of upstream/main” but it starts with a merge of an older commit. If I read the specification of ~ correctly, this will follow the “first parent” of the merge when following the ancestor chain, i.e. the incoming PR gets to choose which of the merge parents is included in -range, by choosing how to order the parents of the merge commit.
There was a problem hiding this comment.
checking out that pr with git pr I see this:
$ git log --oneline HEAD~19..HEAD
8cec77c2 (HEAD -> pr/2876) Test containers_image_sequoia in CI
d2f7e941 Improve test coverage of signature/*_sequoia.go
a79ed910 With sequoia, still use GPGME for existing signing, and add a new Signer API
fd6ca715 Improve test coverage of signature/internal/sequoia/sequoia.go
0af8bd22 Close mechanisms in tests
bee9da73 Direct Rust logging to logrus
b57076c9 Allow using the default Sequoia home
89eaf0f7 Move sequoia initialization out of init()
80364d75 Remove SupportsSigning from sequoia.SigningMechanism
d673d2ac Add a ~representative test of the typical workflow to sequoia.SigningMechanism
2d9475ed Modify signature/internal/sequoia tests to run in the same package
164ef410 Fix a memory leak when loading libpodman_sequoia
6da5b5d5 Add missing error handling to go_sequoia_import_result_get_content .
79eb8840 Don't leak SequoiaMechanism instances
ad7f0391 Update the documentation of sequoiaSigningMechanism.Verify
211ffc85 Allow using sequoia in macOS
89df1aa1 Use github.com/ueno/podman-sequoia instead of a local copy of the code
da9862e5 Merge branch 'wip/signature-sequoia' of https://github.com/ueno/containers-image into signature-sequoia
19834a07 Merge pull request #2937 from containers/renovate/github.com-ulikunitz-xz-0.x
4b07216b Update module github.com/ulikunitz/xz to v0.5.13
a83a1955 signature: add OpenPGP signing mechanism based on Sequoia
So yeah it gets more commits but is also got all commits from the PR so I think that seems fine then?
I mean of course there is the risk that it fails on a commit not in that PR but that should not happen if all PRs were validated before I hope.
Although I need to double check how this interacts with the checkout action as I also have to say checkout X commits there.
There was a problem hiding this comment.
(I’ll try to set up a definite reproducer, but I want to clean my inbox of the re-filed issues first.)
5 participants
see commits