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

[1.9] Disable checkpatch github action on release branch #13904

Closed
wants to merge 1 commit into from

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Nov 5, 2020

checkpatch consistent fails on backport PRs due to the formatting of the
upstream commit, see e.g. [1]. Since we expect changes to go through
master first where checkpatch is run and need to pass before merge,
disable the check on the 1.9 release branch.

[1] https://github.com/cilium/cilium/pull/13903/checks?check_run_id=1357642875

checkpatch consistent fails on backport PRs due to the formatting of the
upstream commit, see e.g. [1]. Since we expect changes to go through
master first where checkpatch is run and need to pass before merge,
disable the check on the 1.9 release branch.

[1] https://github.com/cilium/cilium/pull/13903/checks?check_run_id=1357642875

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
@tklauser tklauser added kind/backports This PR provides functionality previously merged into master. backport/1.9 labels Nov 5, 2020
@tklauser tklauser requested a review from a team as a code owner November 5, 2020 10:24
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Thanks! I noticed that as well and I have a PR (locally) for image-tools to remove GIT_COMMIT_ID from the list of reported issues, this would fix those false positives. I just never finished and pushed it but I'll do it now. Would that be an acceptable fix?

Note that I'm also fine if we remove checkpatch for release branches, since your rational is correct and backport have already been through the checks on the main branch.

@qmonnet
Copy link
Member

qmonnet commented Nov 5, 2020

See cilium/image-tools#85 (and this commit in particular) (this will require an additional PR in Cilium to update the tag of the checkpatch image we use).

@tklauser
Copy link
Member Author

tklauser commented Nov 5, 2020

Thanks! I noticed that as well and I have a PR (locally) for image-tools to remove GIT_COMMIT_ID from the list of reported issues, this would fix those false positives. I just never finished and pushed it but I'll do it now. Would that be an acceptable fix?

Thanks, I'd say fixing this for checkpatch in image-tools is probably the better way forward, as it won't require disabling the GH action in future release branches manually.

Note that I'm also fine if we remove checkpatch for release branches, since your rational is correct and backport have already been through the checks on the main branch.

I'm fine to close this PR and backport an update to the new checkpatch version once cilium/image-tools#85 is merged and updated in cilium/cilium.

@tklauser tklauser closed this Nov 5, 2020
@tklauser tklauser deleted the pr/v1.9-disable-checkpatch-gh-action branch November 5, 2020 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/backports This PR provides functionality previously merged into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants