Skip to content
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

.github: Fix codeQL workflow skip logic #17587

Merged
merged 1 commit into from Oct 13, 2021

Conversation

joestringer
Copy link
Member

Between commits 3ceb742 (".github: Skip unnecessary docs test") and
b08f700 ("workflows: Skip jobs instead of workflows"), we attempted
to add (and fix) logic to skip workflows when the workflow only checks
certain types of code. However, this inadvertently disabled the CodeQL
workflows. Fix it by adjusting the wildcard logic.

Suggested-by: Nicolas Busseneau nicolas@isovalent.com

@joestringer joestringer requested review from a team as code owners October 12, 2021 23:58
@joestringer joestringer added needs-backport/1.10 release-note/ci This PR makes changes to the CI. labels Oct 12, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Oct 12, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.5 Oct 12, 2021
@joestringer
Copy link
Member Author

I've opened this PR from my own tree, and it is demonstrating the current (old) behaviour where the "Deduce required tests..." logic runs, but the "analyze" step does not run:

https://github.com/cilium/cilium/actions/runs/1335318459

I presume that if we merge this, then it will re-enable the "analyze" steps when any Go code changes.

Copy link
Member

@nbusseneau nbusseneau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:yep:

@nbusseneau
Copy link
Member

I've opened this PR from my own tree, and it is demonstrating the current (old) behaviour where the "Deduce required tests..." logic runs, but the "analyze" step does not run:

https://github.com/cilium/cilium/actions/runs/1335318459

I presume that if we merge this, then it will re-enable the "analyze" steps when any Go code changes.

FYI: the workflow uses pull_request so it does use the current version. This can also be seen if you click on the workflow file name at the top of the workflow run (https://github.com/cilium/cilium/actions/runs/1335318459/workflow) which will always display the source code for that workflow run.

The reason it did not run the analyze step for the current PR is simply because it does not have changes to any files matching these filters. If you want to try it out, perhaps made a bogus change in any .go file?

@joestringer
Copy link
Member Author

@joestringer joestringer requested review from a team and jibi October 13, 2021 00:18
@maintainer-s-little-helper maintainer-s-little-helper bot assigned jibi and unassigned jibi Oct 13, 2021
@joestringer joestringer removed the request for review from jibi October 13, 2021 00:18
@joestringer joestringer reopened this Oct 13, 2021
@maintainer-s-little-helper
Copy link

Commit 09136ff4f2f1e40c1b63c326520d7cbf271670d0 does not contain "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Oct 13, 2021
@joestringer
Copy link
Member Author

Between commits 3ceb742 (".github: Skip unnecessary docs test") and
b08f700 ("workflows: Skip jobs instead of workflows"), we attempted
to add (and fix) logic to skip workflows when the workflow only checks
certain types of code. However, this inadvertently disabled the CodeQL
workflows. Fix it by adjusting the wildcard logic to properly match any
files that end with *.go.

Suggested-by: Nicolas Busseneau <nicolas@isovalent.com>
Signed-off-by: Joe Stringer <joe@cilium.io>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Oct 13, 2021
@joestringer joestringer merged commit 852b837 into cilium:master Oct 13, 2021
@joestringer joestringer deleted the submit/gha-fix-codeql branch October 13, 2021 00:44
@joestringer joestringer added this to Needs backport from master in 1.10.6 Oct 13, 2021
@joestringer joestringer removed this from Needs backport from master in 1.10.5 Oct 13, 2021
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.10 in 1.10.6 Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/ci This PR makes changes to the CI.
Projects
No open projects
1.10.6
Backport done to v1.10
Development

Successfully merging this pull request may close these issues.

None yet

4 participants