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

[2024-06-24] Bump dependency updates identified by dependabot #18228

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

@henrybear327
Copy link
Contributor Author

Passing the dependency update torch to @ArkaSaha30 for the entire July after this one! :)

@henrybear327
Copy link
Contributor Author

henrybear327 commented Jun 24, 2024

Updating golangci-lint to 1.59.1 generated the following lint issue

FAIL: (code:1):
  % (cd server && golangci-lint run --config /Users/henrybear327/go/src/etcd/tools/.golangci.yaml ./...)
storage/mvcc/kvstore.go:312:27: (*store).restore - result 0 (error) is always nil (unparam)
func (s *store) restore() error {
                          ^
FAIL: 'run golangci-lint run --config /Users/henrybear327/go/src/etcd/tools/.golangci.yaml ./...' checking failed (!=0 return code)
There was a Failure in module server, aborting...
FAIL: 'lint' FAILED at Mon Jun 24 21:48:39 CEST 2024
make: *** [verify-lint] Error 255

Both Attach() and compactLockfree() within the function restore() are able to return an error but we only log them in the current implementation, thus, the return value for error is always nil, hence the linter warning.

@ahrtr do we need to do anything about this? Or we should just suppress the linter warning here :) Thanks

@ahrtr
Copy link
Member

ahrtr commented Jun 24, 2024

@henrybear327 suggest to only suppress the error for now.

Thanks

Reference:
- etcd-io#18223

Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
Reference:
- etcd-io#18222

Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
golangci-lint reports the following issue:
storage/mvcc/kvstore.go:312:27: (*store).restore - result 0 (error) is always nil (unparam)

It's due to the fact that both Attach() and compactLockfree() within the
function restore() are able to return an error, but we only log them in
the current implementation. Thus, the return value restore() is always
nil, hence the linter warning.

We have agreed to suppress the linter warning for now [1].

Reference:
[1] etcd-io#18228 (comment)

Signed-off-by: Chun-Hung Tseng <henrybear327@gmail.com>
@henrybear327
Copy link
Contributor Author

Both Attach() and compactLockfree() within the function restore() are able to return an error but we only log them in the current implementation, thus, the return value for error is always nil, hence the linter warning.

Done as agreed.

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks

Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks @henrybear327

@jmhbnz jmhbnz merged commit 38535c2 into etcd-io:main Jun 25, 2024
48 checks passed
@henrybear327 henrybear327 deleted the dependency_update/6_24_24 branch June 26, 2024 07:35
@henrybear327
Copy link
Contributor Author

@jmhbnz @ahrtr please close the indirect dependency PRs :) Thanks

@ahrtr
Copy link
Member

ahrtr commented Jun 26, 2024

@jmhbnz @ahrtr please close the indirect dependency PRs :) Thanks

I have been thinking all members have permissions to do that (close dependabot PRs). Can you double check it?

@henrybear327
Copy link
Contributor Author

Screenshot 2024-06-26 at 3 59 39 PM Sadly, no, otherwise I would have done it directly in the past few weeks :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants