Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Remove bootstrap instance warning, show details #6425
Conversation
mjs
changed the title from
Replace bootstrap instance warning
to
Remove bootstrap instance warning, show details
Oct 11, 2016
| + | ||
| +func (s *FormatHardwareSuite) TestNil(c *gc.C) { | ||
| + s.check(c, nil, "") | ||
| + s.check(c, &instance.HardwareCharacteristics{}, "") |
babbageclunk
Oct 11, 2016
Member
You could split this out into TestZero. The various zero tests below are all the same as this case anyway.
mjs
Oct 11, 2016
•
Edited 1 time
-
mjs
Oct 11, 2016
Contributor
-
Oct 11, 2016
Actually it's not the same. TestNil checks what happens when the *HardwareCharacteristics is nil and when there is an instance but all the field values are nil.
The other tests below checks what happens when the fields have a value but they point to the value's zero value.
To make things clearer, I might split up TestNil so that there's TestNil and TestFieldsNil.
| + return strings.Join(out, " ") | ||
| +} | ||
| + | ||
| +func formatMemory(m uint64) string { |
hoenirvili
Oct 11, 2016
Contributor
I think this code seems very familiar.
Check here: https://github.com/juju/juju/blob/master/instance/instance.go#L251
If we are here, instead of writing the same semantic functions to parse memory why not make one global one(move it to utils or smth) and use it in the entire codebase?
What's your thoughts?
mjs
Oct 11, 2016
•
Edited 1 time
-
mjs
Oct 11, 2016
Contributor
-
Oct 11, 2016
The code you link to is for parsing a memory size from a string to an integer. This code is about formatting a memory size (as an integer in megabytes) to a string expressing either megabytes or gigabytes with one decimal place of precision.
I see your point - and I did consider putting it in a more general location - but I think this function is fairly specialised for this use case (and given the 2.0 release cutoff I don't have time to create something more generic right now).
| @@ -200,6 +201,31 @@ func BootstrapInstance(ctx environs.BootstrapContext, env environs.Environ, args | ||
| return result, selectedSeries, finalize, nil | ||
| } | ||
| +func formatHardware(hw *instance.HardwareCharacteristics) string { |
|
$$merge$$ |
|
Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju |
mjs commentedOct 11, 2016
•
Edited 1 time
-
mjs
Oct 11, 2016
The MAAS warning about selection of a default node during bootstrap has been removed and the arch, cores and memory for the bootstrap instance are now shown instead (for all providers).This is generally useful and also informs the user about instance selection decisions made by the provider.
Example output:
QA
Have confirmed the new output is sensible using MAAS, AWS, GCE and LXD.