update golangci-lint version to v1.49.0 #4806
Conversation
k8s-ci-robot
added
cncf-cla: yes
| @@ -204,7 +204,7 @@ func (s *StorageMount) String() string { | |||
| func GetFunctionSpec(n *yaml.RNode) (*FunctionSpec, error) { | |||
| meta, err := n.GetMeta() | |||
| if err != nil { | |||
| return nil, nil | |||
| return nil, fmt.Errorf("failed to get ResourceMeta: %w", err) | |||
There was a problem hiding this comment.
Although these may be good changes, they look unrelated to this PR. Is a new linter somehow triggering the need for them?
There was a problem hiding this comment.
It was caused by the nilerr with the new version.
That is not a new linter. That linter was enabled before creating this PR.
That lint would show the below errors if this line weren't fixed.
error is not nil (line 205) but it returns nil (nilerr)
There was a problem hiding this comment.
Ah, I see. This was probably done to make the function alpha as unlikely as possible to generate errors, but now that we're trying to move functions forward, I think you're right to return the error here. It would be very surprising to get an error here, and if somehow that happened, we don't know whether or not the metadata we failed to retrieve contained a function.
|
Hi, @KnVerey Thanks to check this PR. |
| @@ -204,7 +204,7 @@ func (s *StorageMount) String() string { | |||
| func GetFunctionSpec(n *yaml.RNode) (*FunctionSpec, error) { | |||
| meta, err := n.GetMeta() | |||
| if err != nil { | |||
| return nil, nil | |||
| return nil, fmt.Errorf("failed to get ResourceMeta: %w", err) | |||
There was a problem hiding this comment.
Ah, I see. This was probably done to make the function alpha as unlikely as possible to generate errors, but now that we're trying to move functions forward, I think you're right to return the error here. It would be very surprising to get an error here, and if somehow that happened, we don't know whether or not the metadata we failed to retrieve contained a function.
| @@ -281,7 +281,8 @@ metadata: | |||
| out: []string{"gcr.io/example.com/image:v1.0.0"}, | |||
| }, | |||
|
|
|||
| {name: "no function spec", | |||
| { | |||
| name: "no function spec", | |||
There was a problem hiding this comment.
Based on this title, I think this case is meant to ensure that we exit successfully when given a resource that simply does not contain a function. Unfortunately it didn't use a valid resource, so it is surfacing the error now. I think it is good to have the error test case too though! Would you mind changing the title here to something like "invalid input object" and restoring the original case but with a well-formed object?
There was a problem hiding this comment.
Unfortunately it didn't use a valid resource, so it is surfacing the error now.
Oh, I didn't find that.
I restored the original test case at 6ce230f.
|
@koba1t: This PR has multiple commits, and the default merge method is: merge. 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. |
k8s-ci-robot
added
size/L
|
Hi, @KnVerey |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: KnVerey, koba1t 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 |
k8s-ci-robot
added
the
approved
Update golangci-lint v1.46.2 to v1.49.0
This update is improving the new go version support.
additional
below new linters are added
delete varcheck, structcheck, and deadcode because there are deprecated.