Skip to content
This repository has been archived by the owner on Jul 16, 2024. It is now read-only.

Winter code cleanup #349

Merged
merged 4 commits into from
Dec 9, 2022
Merged

Conversation

hakman
Copy link
Contributor

@hakman hakman commented Dec 6, 2022

Some issues still remain, but very few:

> staticcheck -- ./... | grep -v -e "\.pb\.go" -e "proto" 
pkg/controller/controller.go:779:28: this result of append is never used, except maybe in other appends (SA4010)
pkg/etcdclient/client.go:144:2: empty branch (SA9003)
pkg/hostmount/nsenter.go:47:15: ListProcMounts not declared by package mount (compile)
pkg/hostmount/nsenter.go:94:21: MakeMountArgs not declared by package mount (compile)
pkg/hostmount/nsenter.go:112:31: AddSystemdScope not declared by package mount (compile)
pkg/pki/fs_test.go:126:28: actual.CertPool().Subjects has been deprecated since Go 1.18: if s was returned by SystemCertPool, Subjects will not include the system roots.  (SA1019)
pkg/privateapi/peers.go:301:23: grpc.WithInsecure is deprecated: use WithTransportCredentials and insecure.NewCredentials() instead. Will be supported throughout 1.x.  (SA1019)
pkg/privateapi/peers.go:304:22: grpc.WithBackoffMaxDelay is deprecated: use WithConnectParams instead. Will be supported throughout 1.x.  (SA1019)

/cc @olemarkus @justinsb

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman

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

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Dec 6, 2022
@hakman hakman force-pushed the cleanup branch 3 times, most recently from dcb0130 to 633f61d Compare December 6, 2022 17:48
@@ -950,13 +950,13 @@ func (m *EtcdController) removeNodeFromCluster(ctx context.Context, clusterSpec
peerState := m.peerState[privateapi.PeerId(id)]
if peerState == nil {
klog.Fatalf("peerState unexpectedly nil")
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

This change seems less desirable than what was before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

More or less agree, but staticheck was seeing a possible issue with the code. It's not so bad as it is now either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any objection to merging this, now that this has been reverted?

@hakman hakman requested review from johngmyers and removed request for olemarkus and justinsb December 9, 2022 04:04
@johngmyers
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 9, 2022
@k8s-ci-robot k8s-ci-robot merged commit 4858ad1 into kubernetes-retired:master Dec 9, 2022
@hakman hakman deleted the cleanup branch December 9, 2022 05:01
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants