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

Windows-cross linting #21441

Merged
merged 3 commits into from
Feb 9, 2024
Merged

Windows-cross linting #21441

merged 3 commits into from
Feb 9, 2024

Conversation

cevich
Copy link
Member

@cevich cevich commented Jan 30, 2024

Depends on: #21372

Performing native linting on Windows is quite complex and will require updating the winmake PS script. As a stop-gap, and until we have an entirely native build -> test chain, run Windows linting prior to the cross-build (on Linux). This isn't a perfect solution, but it does find lots of lint that requires cleaning. To that end, enforcement of cleanliness is temporarily bypassed by in a single revert-friendly commit.

Does this PR introduce a user-facing change?

None

@openshift-ci openshift-ci bot added release-note-none do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jan 30, 2024
@cevich cevich mentioned this pull request Jan 30, 2024
@cevich cevich force-pushed the win_lint branch 2 times, most recently from c65477a to 3e4c62c Compare February 1, 2024 14:48
@cevich cevich marked this pull request as ready for review February 1, 2024 14:51
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 4, 2024
@cevich cevich changed the title [WIP] [CI:MACHINE] Windows-cross linting [CI:MACHINE] Windows-cross linting Feb 6, 2024
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 6, 2024
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 6, 2024
@cevich cevich requested review from baude, edsantiago and mheon February 6, 2024 14:16
@cevich cevich changed the title [CI:MACHINE] Windows-cross linting Windows-cross linting Feb 6, 2024
@cevich
Copy link
Member Author

cevich commented Feb 6, 2024

force-push: Rebased. Assuming CI passes, this should be ready to go.

hack/golangci-lint.sh Outdated Show resolved Hide resolved
Specifically, Darwin's bash is very old so it doesn't support newer
features like `declare -A`.  Reduce the complexity of the script
so that it can be used for all platforms.  Comment heavily regarding the
scripts various execution contexts to prevent developers relying on
advanced features for any future modifications.

Signed-off-by: Chris Evich <[email protected]>
As of this commit, there are several pages worth of lint findings for
windows.  Once they're all addressed, this commit may be reverted to
enable continuous checking.

Signed-off-by: Chris Evich <[email protected]>
@edsantiago
Copy link
Member

LGTM but I won't merge until we get the all-clear from @baude

@edsantiago
Copy link
Member

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 9, 2024
Copy link
Contributor

openshift-ci bot commented Feb 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, edsantiago

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 9, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit e16d82d into containers:main Feb 9, 2024
85 of 89 checks passed
@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label May 10, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators May 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants