[VPA] Enable multiple revive rules - 4#9155
Conversation
|
Hi @mayuka-c. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
| admissionv1 "k8s.io/api/admission/v1" | ||
| apiv1 "k8s.io/api/core/v1" | ||
| v1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" |
There was a problem hiding this comment.
Based on the comments here, have enabled the "redundant-import-alias" rule
|
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
|
Hmmm, I guess the generated files should be excluded from the linting |
c9230c9 to
18a813a
Compare
Yes right. Currently I have not changed anything in generated files (Didnt face any lint rule issues) for now. Maybe later we can try excluding linter rules on Generated file? |
|
@adrianmoisey @omerap12 Have rebased the PR. PTAL |
|
/ok-to-test |
| "fmt" | ||
|
|
||
| v1 "k8s.io/api/admission/v1" | ||
| corev1 "k8s.io/api/core/v1" |
There was a problem hiding this comment.
I'm taking a look at the kubernetes repo, and it seems that this package is often called corev1, any reason why this changes to apiv1 ?
There was a problem hiding this comment.
When I looked across the repo, most of the places we have used apiv1, so I have used this to make it consistent.
PLMK if we want to have corev1
There was a problem hiding this comment.
corev1 would be great, thanks
There was a problem hiding this comment.
WIll be making the changes, Thanks!
There was a problem hiding this comment.
Done made the updates
| "context" | ||
|
|
||
| v1 "k8s.io/api/admission/v1" | ||
| apiv1 "k8s.io/api/admission/v1" |
There was a problem hiding this comment.
Should this be admissionv1 ?
There was a problem hiding this comment.
oops my bad, I will update it
|
So this change is huge, but something I really want merged. It's likely that when this PR lands, many other PRs will require a rebase, I chatted a bit to Omer about this, and we decided to only merge this after the CPU boost PR has landed. That PR has been in the works for a while now and is code complete, so it should be merged in the next day or two. This may cause merge conflicts on your PR (sorry about that!), but we can try get it merged in after that. |
184ab59 to
9acace5
Compare
|
Looks like few things got messed up while rebasing, let me try to fix it |
9acace5 to
34c5c9b
Compare
34c5c9b to
64db58d
Compare
|
@adrianmoisey PR is ready! |
|
Let's do it! /lgtm /cc @omerap12 |
|
/approve (for hack/OWNERS) |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adrianmoisey, mayuka-c, omerap12 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Enables multiple revive rules.
Below are the revive linter rules enabled as part of this PR
- name: redefines-builtin-id
- name: redundant-import-alias
Which issue(s) this PR fixes:
Updates #8986
Special notes for your reviewer:
Continuation from this PR.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: