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

✨ Import groups linting #240

Closed
wants to merge 2 commits into from

Conversation

trgeiger
Copy link
Contributor

@trgeiger trgeiger commented Apr 9, 2024

Adds linter grouping to match configuration in operator-controller.

golangci-lint will now group imports in the following order:
- standard library
- 3rd party packages outside of our Github org
- packages from our Github org
- local imports from catalogd

Signed-off-by: Tayler Geiger <tayler@redhat.com>
Signed-off-by: Tayler Geiger <tayler@redhat.com>
@trgeiger trgeiger requested a review from a team as a code owner April 9, 2024 21:20
Copy link

codecov bot commented Apr 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 48.84%. Comparing base (0d07c30) to head (ca8e8bc).
Report is 10 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #240   +/-   ##
=======================================
  Coverage   48.84%   48.84%           
=======================================
  Files           8        8           
  Lines         434      434           
=======================================
  Hits          212      212           
  Misses        201      201           
  Partials       21       21           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- standard
- default
- prefix(github.com/operator-framework)
- prefix(github.com/operator-framework/catalogd)
Copy link
Member

Choose a reason for hiding this comment

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

Use localmodule here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like localmodule is only working in gci 0.13.4 which isn't included in a release of golangci-lint yet.

"github.com/spf13/pflag"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
Copy link
Member

Choose a reason for hiding this comment

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

Seems like we have conflicting linters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's due to the following comments on 32-33. Removing those fixes the incompatibility.

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

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/test-infra repository.

@everettraven
Copy link
Collaborator

Looks like this will need a rebase if we want to still get this in

@trgeiger
Copy link
Contributor Author

Looks like @joelanford already knocked this out here: #255

@trgeiger trgeiger closed this May 23, 2024
auto-merge was automatically disabled May 23, 2024 20:59

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants