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

Use staticcheck #201

Merged
merged 11 commits into from
Nov 4, 2020
Merged

Use staticcheck #201

merged 11 commits into from
Nov 4, 2020

Conversation

ysksuzuki
Copy link
Contributor

@ysksuzuki ysksuzuki commented Oct 30, 2020

This PR modifies the followings

  • Use staticcheck instead of golint
  • Flatten mtest directory structure to avoid staticcheck warning.
  • Fix other staticcheck errors
  • Return error when a user passes an invalid sabactl subcommand.

@ysksuzuki ysksuzuki self-assigned this Oct 30, 2020
@masa213f masa213f force-pushed the refactor-mtest branch 4 times, most recently from 5a8f843 to 31b1f04 Compare October 30, 2020 06:30
Signed-off-by: Masayuki Ishii <masa213f@gmail.com>
Signed-off-by: Masayuki Ishii <masa213f@gmail.com>
Signed-off-by: Masayuki Ishii <masa213f@gmail.com>
Signed-off-by: Masayuki Ishii <masa213f@gmail.com>
Signed-off-by: Masayuki Ishii <masa213f@gmail.com>
Signed-off-by: Masayuki Ishii <masa213f@gmail.com>
Signed-off-by: Yusuke Suzuki <yusuke-suzuki@cybozu.co.jp>
@ysksuzuki ysksuzuki marked this pull request as ready for review October 30, 2020 09:01
@masa213f masa213f changed the title refactor mtest Use staticcheck Nov 4, 2020
@@ -19,21 +19,15 @@ import (

const (
// ExitSuccess represents no error.
ExitSuccess subcommands.ExitStatus = subcommands.ExitSuccess
Copy link
Member

Choose a reason for hiding this comment

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

Please get rid of github.com/google/subcommands as it was replaced with cobra.

Comment on lines 70 to 72
if l.hwMap == nil {
return json.Marshal(make(map[string]leaseInfo))
}
Copy link
Member

Choose a reason for hiding this comment

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

unnecessary.

mtest/Makefile Outdated Show resolved Hide resolved
ysksuzuki and others added 3 commits November 4, 2020 11:17
Co-authored-by: Yamamoto, Hirotaka <ymmt2005@gmail.com>
Signed-off-by: Yusuke Suzuki <yusuke-suzuki@cybozu.co.jp>
Signed-off-by: Yusuke Suzuki <yusuke-suzuki@cybozu.co.jp>
if l.hwMap == nil {
l.hwMap = make(map[string]leaseInfo)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In some cases, hwmap will be nil, so we initialize it here.

Copy link
Member

Choose a reason for hiding this comment

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

Why do you want to initialize it here?
I think it can be initialized where a new key is stored in this map.

Copy link
Contributor

@masa213f masa213f Nov 4, 2020

Choose a reason for hiding this comment

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

I think it is better to allocate the map when initializing the "leaseUsage" structure.
In this case, in effect, this unmarshaller initializes the structure, so we're allocating the map here.

Signed-off-by: Masayuki Ishii <masa213f@gmail.com>
Copy link
Member

@ymmt2005 ymmt2005 left a comment

Choose a reason for hiding this comment

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

LGTM

@masa213f masa213f merged commit cb552fd into master Nov 4, 2020
@masa213f masa213f deleted the refactor-mtest branch November 4, 2020 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants