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.

Sign up for GitHub

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

Jump to bottom

HOSTEDCP-2176: Add golangci-lint to verify in Makefile #4739

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

HOSTEDCP-2176: Add golangci-lint to verify in Makefile #4739

wants to merge 7 commits into from

Conversation

bryan-cox
Copy link
Member

@bryan-cox bryan-cox commented Sep 17, 2024 •
edited
Loading

What this PR does / why we need it:
Add golangci-lint to verify in Makefile so common mistakes are caught in presubmit PRs.

This PR also introduces a way to enforce Golang imports are properly formatted throughout the project.

Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

@bryan-cox
Copy link
Member Author

/test verify

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 17, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Sep 17, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Sep 17, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bryan-cox

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 Sep 17, 2024
@bryan-cox
Copy link
Member Author

/test verify

@openshift-ci openshift-ci bot added the area/cli Indicates the PR includes changes for CLI label Sep 17, 2024
@bryan-cox
Copy link
Member Author

/test verify

@openshift-ci openshift-ci bot added area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/control-plane-pki-operator Indicates the PR includes changes for the control plane PKI operator - in an OCP release area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release area/testing Indicates the PR includes changes for e2e testing and removed do-not-merge/needs-area labels Sep 17, 2024
@bryan-cox
Copy link
Member Author

/test verify

@bryan-cox bryan-cox force-pushed the golang-lint branch 3 times, most recently from d47de39 to 2c50d0b Compare September 18, 2024 18:58
@netlify Netlify
Copy link

netlify bot commented Sep 25, 2024 •
edited
Loading

✅ Deploy Preview for hypershift-docs ready!

Name Link
🔨 Latest commit b2220b3
🔍 Latest deploy log https://app.netlify.com/sites/hypershift-docs/deploys/673e42f57e6d2c0008e74822
😎 Deploy Preview https://deploy-preview-4739--hypershift-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 26, 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 Oct 29, 2024
@bryan-cox bryan-cox force-pushed the golang-lint branch 2 times, most recently from 94cdc05 to 8792d5e Compare October 29, 2024 17:07
@bryan-cox
Copy link
Member Author

/test all

@bryan-cox
Copy link
Member Author

/test all

1 similar comment
@bryan-cox
Copy link
Member Author

/test all

@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 13, 2024 •
edited by openshift-ci bot
Loading

@bryan-cox: This pull request references HOSTEDCP-2176 which is a valid jira issue.

In response to this:

What this PR does / why we need it:
Add golangci-lint to verify in Makefile so common mistakes are caught in presubmit PRs.

This PR also introduces a way to enforce Golang imports are properly formatted throughout the project.

Which issue(s) this PR fixes (optional, use fixes #<issue_number>(, fixes #<issue_number>, ...) format, where issue_number might be a GitHub issue, or a Jira story:
Fixes #

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@bryan-cox
Copy link
Member Author

There are additional linters we can add in the future if we want - https://golangci-lint.run/usage/linters/

@bryan-cox
Copy link
Member Author

/retest

1 similar comment
@bryan-cox
Copy link
Member Author

/retest

@bryan-cox
Copy link
Member Author

/retest-required

1 similar comment
@bryan-cox
Copy link
Member Author

/retest-required

@bryan-cox
Copy link
Member Author

/test verify

@bryan-cox
Copy link
Member Author

/retest

@bryan-cox bryan-cox force-pushed the golang-lint branch 2 times, most recently from b2220b3 to e376517 Compare November 21, 2024 13:01
@bryan-cox
Copy link
Member Author

/retest-required

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2024
@bryan-cox
Add golangci-lint to verify in Makefile
89fc87b 
Add golangci-lint to verify in Makefile so common mistakes are caught in
 presubmit PRs.

Signed-off-by: Bryan Cox <brcox@redhat.com>
@bryan-cox
Update hack/tools/go.mod to include golangci-lint
87de266 
Signed-off-by: Bryan Cox <brcox@redhat.com>
@bryan-cox
Fix golangci-lint errors
34f8aad 
Signed-off-by: Bryan Cox <brcox@redhat.com>
@bryan-cox
Enable GCI to sort imports
7405751 
Signed-off-by: Bryan Cox <brcox@redhat.com>
@bryan-cox
Sort imports in HyperShift project
1a96a12 
Signed-off-by: Bryan Cox <brcox@redhat.com>
@bryan-cox
Fix delegating_client.go issue
f92a162 
Signed-off-by: Bryan Cox <brcox@redhat.com>
@bryan-cox
Add tools go files to codespell ignore
8decbb2 
Signed-off-by: Bryan Cox <brcox@redhat.com>
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 13, 2024
@bryan-cox
Copy link
Member Author

/retest

@bryan-cox
Copy link
Member Author

/retest-required

2 similar comments
@bryan-cox
Copy link
Member Author

/retest-required

@bryan-cox
Copy link
Member Author

/retest-required

@bryan-cox
Copy link
Member Author

/retest

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Dec 17, 2024

@bryan-cox: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-kubevirt-aws-ovn-reduced 8decbb2 link true /test e2e-kubevirt-aws-ovn-reduced
ci/prow/okd-scos-e2e-aws-ovn 8decbb2 link false /test okd-scos-e2e-aws-ovn
ci/prow/e2e-aws 8decbb2 link true /test e2e-aws

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 17, 2024
@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@bryan-cox bryan-cox mentioned this pull request Jan 7, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@csrwng csrwng Awaiting requested review from csrwng

@davidvossel davidvossel Awaiting requested review from davidvossel

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/api Indicates the PR includes changes for the API area/ci-tooling Indicates the PR includes changes for CI or tooling area/cli Indicates the PR includes changes for CLI area/control-plane-operator Indicates the PR includes changes for the control plane operator - in an OCP release area/control-plane-pki-operator Indicates the PR includes changes for the control plane PKI operator - in an OCP release area/hypershift-operator Indicates the PR includes changes for the hypershift operator and API - outside an OCP release area/testing Indicates the PR includes changes for e2e testing jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bryan-cox @openshift-ci-robot @openshift-merge-robot