-
Notifications
You must be signed in to change notification settings - Fork 62
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
Delete Makefile checks from GitHub actions #262
Delete Makefile checks from GitHub actions #262
Conversation
9ef4248
to
254a708
Compare
/cc @mauriciopoppe |
.github/workflows/makefile.yml
Outdated
@@ -0,0 +1,21 @@ | |||
name: Makefile Checks |
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.
Please rename this file to code format checks, also add a comment at the top explaining why we need a linux job in addition to the Windows job
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.
Done! Force-pushed to squash.
254a708
to
a6e8363
Compare
uses: actions/checkout@v2 | ||
- name: Run Code Format Checks | ||
run: | | ||
make test-fmt && |
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.
there was the subtree check too right? I think this test that release-tools wasn't modified and that I think is important since this folder comes from a different repo.
could we run make all
as it is in Linux instead?
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.
test-subtree
was failing in my previous attempt to refactor the GitHub actions: PR #261. In fact I think this issue would have been happening all along, but the error was not showing because make all
fails on test-fmt
and never got to execute test-subtree
.
The reason behind this is that GitHub Action's checkout@v2 grabs the latest commit as a graft, which causes the git log -- release-tools
command run by verify-subtree to think that the last commit added the entire repository (or something like that).
I did some digging, and it seems like no other repo in CSI Proxy runs verify-subtree in a GitHub action. I think we can just remove it and run make test-fmt test-vendor test-shellcheck
.
/retest |
88a37af
to
17f8a02
Compare
- name: Checkout code | ||
uses: actions/checkout@v2 | ||
with: | ||
fetch-depth: 0 |
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, I was reading the logs and there's this job which is already doing the same I think pull-kubernetes-csi-csi-proxy
, I didn't check the full log though
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 you're right. Should we just delete this then?
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.
yes please, removing make all
from the windows job is great though
17f8a02
to
0d6ebaf
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexander-ding, mauriciopoppe The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
98f2307 Merge pull request kubernetes-csi#260 from TerryHowe/update-csi-driver-version e9d8712 Merge pull request kubernetes-csi#259 from stmcginnis/deprecated-kind-kube-root faf79ff Remove --kube-root deprecated kind argument 734c2b9 Merge pull request kubernetes-csi#265 from Rakshith-R/consider-main-branch f95c855 Merge pull request kubernetes-csi#262 from huww98/golang-toolchain 3c8d966 Treat main branch as equivalent to master branch e31de52 Merge pull request kubernetes-csi#261 from huww98/golang fd153a9 Bump golang to 1.23.1 a8b3d05 pull-test.sh: fix "git subtree pull" errors 6b05f0f use new GOTOOLCHAIN env to manage go version 18b6ac6 chore: update CSI driver version to 1.15 git-subtree-dir: release-tools git-subtree-split: 98f23071d946dd3de3188a7e1f84679067003162
What type of PR is this?
What this PR does / why we need it:
GitHub action test is overlapping with Prow.
Which issue(s) this PR fixes:
Fixes #259
Special notes for your reviewer:
Reopening to trigger the GH action job again.
Does this PR introduce a user-facing change?: