Skip to content
This repository was archived by the owner on Oct 24, 2023. It is now read-only.

style: remove unneeded nil check#1110

Merged
jackfrancis merged 2 commits intoAzure:masterfrom
mboersma:fix-govet-whining
Apr 22, 2019
Merged

style: remove unneeded nil check#1110
jackfrancis merged 2 commits intoAzure:masterfrom
mboersma:fix-govet-whining

Conversation

@mboersma
Copy link
Member

@mboersma mboersma commented Apr 22, 2019

Reason for Change:

In master there is some (old) code that fails govet with golangci-lint v1.16.0. This removes a redundant nil check to satisfy it. Look at the code just above the diff to see why it's redundant.

==> Running static validations and linters <==
pkg/api/converterfromagentpoolonlyapi.go:148:8: nilness: tautological condition: non-nil != nil (govet)
	if kc != nil && kc.EnableRbac != nil && *kc.EnableRbac {
	      ^
Makefile:138: recipe for target 'test-style' failed
make: *** [test-style] Error 1

Issue Fixed:

Unblocks #1088

Requirements:

Notes:
I'm not sure how this made it into master without us noticing. The code hasn't been changed, and the linter version changed a while ago...

@codecov
Copy link

codecov bot commented Apr 22, 2019

Codecov Report

Merging #1110 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1110      +/-   ##
==========================================
- Coverage   74.35%   74.35%   -0.01%     
==========================================
  Files         131      131              
  Lines       18274    18270       -4     
==========================================
- Hits        13588    13584       -4     
  Misses       3905     3905              
  Partials      781      781

}
// We use KubernetesConfig.EnableRbac to convert to versioned api model
// The assumption here is KubernetesConfig.EnableSecureKubelet is set to be same
if kc != nil && kc.EnableRbac != nil && *kc.EnableRbac {
Copy link
Contributor

Choose a reason for hiding this comment

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

slight preference for removing the above check instead so we can have one return to.BoolPtr(false) instead of two.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or even better:

return to.BoolPtr(kc != nil && kc.EnableRbac != nil && *kc.EnableRbac)

Copy link
Contributor

Choose a reason for hiding this comment

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

yes!! 😍

@jackfrancis
Copy link
Member

lgtm

Copy link
Contributor

@CecileRobertMichon CecileRobertMichon left a comment

Choose a reason for hiding this comment

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

/lgtm

@acs-bot
Copy link

acs-bot commented Apr 22, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon, mboersma

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [CecileRobertMichon,mboersma]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jackfrancis jackfrancis merged commit 18020fd into Azure:master Apr 22, 2019
@mboersma mboersma deleted the fix-govet-whining branch June 21, 2019 19:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants