Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
environ.Instances and .AllInstances implemented for MAAS 2 #4995
Conversation
and others
added some commits
Apr 1, 2016
natefinch
reviewed
Apr 5, 2016
| + } else { | ||
| + filter := getSystemIdValues("id", ids) | ||
| + filter.Add("agent_name", environ.ecfg().maasAgentName()) | ||
| + return environ.instances1(filter) |
natefinch
Apr 5, 2016
Contributor
I'd put the maas1 up top, since it's shorter, and then drop the else and unindent, since you're returning in the first block.
|
other than one nitpick, LGTM |
babbageclunk
added some commits
Apr 5, 2016
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
jujubot
merged commit 26ac1eb
into
juju:maas2
Apr 6, 2016
babbageclunk
deleted the
babbageclunk:maas2-acquired-instances
branch
Apr 25, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
babbageclunk commentedApr 5, 2016
Rather than branching in AllInstances, push the API version branching down into acquiredInstances.
Corrected some error handling in Instances - before it wouldn't return ErrPartialInstances if the set of ids that came back was different but had the same size.