-
Notifications
You must be signed in to change notification settings - Fork 161
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
Fix golint errors in lib/selector #430
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for cleaning up this code!
I've made a few minor comments on some typos. Worth @fasaxc having a quick look through as he knows this bit of code better than I do.
lib/selector/parser/ast.go
Outdated
String() string | ||
UniqueId() string | ||
// String returns the unique ID that represents this selector. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
String -> UniqueID
lib/selector/parser/ast.go
Outdated
type LabelInSetNode struct { | ||
LabelName string | ||
Value StringSet | ||
} | ||
|
||
// Evaluate return true if the labels' value in the set of current LabelInSetNode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: return -> returns
lib/selector/parser/ast.go
Outdated
type LabelNotInSetNode struct { | ||
LabelName string | ||
Value StringSet | ||
} | ||
|
||
// Evaluate return true if the labels' value not in the set of current LabelNotInSetNode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return -> returns
lib/selector/parser/ast.go
Outdated
type LabelNeValueNode struct { | ||
LabelName string | ||
Value string | ||
} | ||
|
||
// Evaluate return true if the labels' value not equal to the value of current LabelNeValueNode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return -> returns
lib/selector/parser/ast.go
Outdated
type HasNode struct { | ||
LabelName string | ||
} | ||
|
||
// Evaluate return true if the labels has the LabelName of current HasNode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return -> returns (and a few more through the PR)
lib/selector/parser/stringset.go
Outdated
@@ -49,6 +51,7 @@ func (ss StringSet) Contains(s string) bool { | |||
return ss[minIdx] == s | |||
} | |||
|
|||
// ConvertToStringSetInPlace convert a string slice into StringSet in place |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
convert -> converts
lib/selector/tokenizer/tokenizer.go
Outdated
@@ -70,6 +88,7 @@ var ( | |||
inRegex = regexp.MustCompile("^" + inExpr) | |||
) | |||
|
|||
// Tokenize transform input string to tokens |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transform -> transforms
b1e5b7b
to
b56deb6
Compare
@xychu You code cleanups/name fixes look good to me but a lot of the comments are of the form:
I'd prefer to have no comment in that case since the comment doesn't add very much. What do you think? |
@fasaxc sure, will update it later. |
@fasaxc @caseydavenport plz review again. |
Not sure if this is the right time to do the golint checks. :-)