-
Notifications
You must be signed in to change notification settings - Fork 8
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
[sabakan] improve sabactl machines get #239
Conversation
29685f5
to
ee21c7c
Compare
Signed-off-by: yamatcha <soju-yamashita@cybozu.co.jp>
Signed-off-by: yamatcha <soju-yamashita@cybozu.co.jp>
Signed-off-by: yamatcha <soju-yamashita@cybozu.co.jp>
Please add test cases. |
@@ -152,8 +153,10 @@ func (mi *machinesIndex) query(q sabakan.Query) []string { | |||
|
|||
res := make(map[string]struct{}) | |||
|
|||
for _, serial := range mi.Rack[q.Rack()] { | |||
res[serial] = struct{}{} | |||
for _, rack := range strings.Split(q.Rack(), ",") { |
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.
Why can we only specify multiple racks?
We want to specify other options as well.
The original issue is just an example of a rack.
models/etcd/machine.go
Outdated
@@ -258,8 +258,9 @@ RETRY: | |||
func (d *driver) machineQuery(ctx context.Context, q sabakan.Query) ([]*sabakan.Machine, error) { | |||
var serials []string | |||
|
|||
qRemovedWithout := q.RemoveWithout() |
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.
I could not read the intent of this code.
Does "without" mean the same as "empty"?
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.
I understood.
But I think this code is not readable.
Please rewrite this code or add a comment.
I checked for now! |
Signed-off-by: yamatcha <soju-yamashita@cybozu.co.jp>
Signed-off-by: yamatcha <soju-yamashita@cybozu.co.jp>
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.
LGTM
Signed-off-by: yamatcha soju-yamashita@cybozu.co.jp