Windows: Fix camelCase inconsistencies #859

lowenna · 2017-05-25T21:55:44Z

Signed-off-by: John Howard jhoward@microsoft.com

Addresses credentialSpec, utilityVMPath, and ignoreFlushesDuringBoot.

REMOVED ~~~I noticed there were discrepancies also, particularly in the SolarisAnet structure (camelCase and non-matching field names to json names), so I fixed these up at the same time.~~~

wking

Ping @anuthan for feedback on the Solaris changes.

wking · 2017-05-25T22:15:14Z

config-solaris.md

@@ -12,15 +12,15 @@ The SMF(Service Management Facility) FMRI which should go to "online" state befo
 "milestone": "svc:/milestone/container:default"
 ```

-## <a name="configSolarisLimitpriv" />limitpriv
+## <a name="configSolarisLimitPriv" />limitpriv


If you're changing limitpriv to limitPriv below, I expect you want to change it in the header here as well. You've currently changed the anchor, but not the visible text.

Ugh, sorry, I missed that one. Force update coming in a minute.

wking · 2017-05-25T22:19:30Z

config-solaris.md

@@ -90,7 +90,7 @@ Mapped to `allowed-address` in the [zonecfg(1M)][zonecfg.1m_2] man page.
 * **`configureAllowedAddress`** *(string, OPTIONAL)* If configureAllowedAddress is set to true, the addresses specified by allowedAddress are automatically configured on the interface each time the container starts.
 When it is set to false, the allowedAddress will not be configured on container start.
 Mapped to `configure-allowed-address` in the [zonecfg(1M)][zonecfg.1m_2] man page.
-* **`defrouter`** *(string, OPTIONAL)* The value for the OPTIONAL default router.
+* **`defaultRouter`** *(string, OPTIONAL)* The value for the OPTIONAL default router.


It seems inconsistent to me to expand def → default here but not expand Priv → Privilege. I personally prefer the unabbreviated names, but am not sure how the Solaris folks want to handle this. I'm fine with this expansion and/or the Priv → Privilege expansion happening in a follow-up PR if we want to isolate those changes from the camelCase changes.

The link in the solaris markdown was wrong, but I'm updating it to http://docs.oracle.com/cd/E19253-01/816-5168/priv-str-to-set-3c/index.html as that seems the correct link.

However, as from that link the documentation seems to use Priv, I'm leaving it as-is.

As for def or default, def seems a very unusual abbreviation. Using default is also consistent with defaultAction. There seems no point that I can see for abbreviating it.

I take it back, simply on account of it being def in the Solaris documentation. Reverted that change.

I take it back, simply on account of it being def in the Solaris documentation.

“Expose the platform abbreviation” sounds like a sane way to frame the policy. But then perhaps we want “expose the platform casing” as well, in which case we'd want to leave this entry as defrouter and not touch it in this PR.

hey @wking and @jhowardmsft. i'll be taking over responsibility for the solaris components from @anuthan. i've reviewed our use of camel case and i don't actually want to make any changes to it. in most cases our property names mirror what's found in zonecfg ("defrouter" being an example), and camel case was only used for property names that had a hyphen in them. for example, the zonecfg "allowed-address" property is mapped to "allowedAddress". hopefully that sounds reasonable?

that said, some of our docs.oracle.com links are broken so i'll file and issue for that and get it fixed.

wking · 2017-05-25T22:27:58Z

specs-go/config.go

@@ -495,7 +495,7 @@ type WindowsNetwork struct {
 // WindowsHyperV contains information for configuring a container to run with Hyper-V isolation.
 type WindowsHyperV struct {
 	// UtilityVMPath is an optional path to the image used for the Utility VM.
-	UtilityVMPath string `json:"utilityvmpath,omitempty"`
+	UtilityVMPath string `json:"utilityVMPath,omitempty"`


On the unabbreviation front, this could be utilityVirtualMachinePath. I don't see a way to draw a hard line for this, and it's orthogonal to camelCasing, but I thought I'd point out that def → default style expansion is something of a slippery slope. My own fuzzy possition is that I like def → default but am less enthusiastic about VM → VirtualMachine. And while I think blkioThrottleWriteIOPSDevice is a bit hard to read, it may be a better choice than throttleWriteOperationsPerSecondPerDevice. But if anyone does see a way to draw a good line, it would be nice to document that line in style.md.

Yes, I agree it's a fine line, but IMO, I would say VM is a very common abbreviation for 'Virtual Machine' and think it's OK as is.

FYI, I no longer work on containers, I will find a point of contact from Solaris for you guys.
Sorry about this.

hey @wking and @jhowardmsft. i'll be taking over responsibility for the solaris containers work from @anuthan. i did an audit of our use of camel case and i'd prefer not to change it. in general we've tried to have our property names map to zonecfg.1m properties. "defrouter" being a good example. the places where we have used camel case is for zonecfg properties that have hyphens in them. so for example the zonecfg "allowed-address" property has been mapped to allowedAddress. hopefully this seems sensible to folks.

that said, john correctly noted that some of our doc links are broken so i filed issue #864 on that.

lowenna · 2017-05-25T23:13:19Z

OK, I don't want to rathole for 1.0, when what I was originally intending fixing which was the camelCasing in Windows. Reverted all Solaris changes and that can be taken up separately if necessary.

wking · 2017-05-25T23:19:17Z

On Thu, May 25, 2017 at 04:13:20PM -0700, John Howard wrote: Reverted all Solaris changes and that can be taken up separately if necessary.

That sounds reasonable to me, with the policy being something like “use whatever contributors interested in platform can get the maintainers to agree to” ;). And 451856a looks like it covers all the camelCasing for config-windows.md, although there are still references in the JSON Schema that need updating.

Signed-off-by: John Howard <jhoward@microsoft.com>

lowenna · 2017-05-25T23:23:20Z

And 451856a looks like it covers all the camelCasing for
config-windows.md, although there are still references in the JSON
Schema that need updating.

Thanks. Those too are updated now at c6338b3

TomSweeneyRedHat · 2017-05-26T12:11:13Z

LGTM

mrunalp · 2017-05-31T21:54:18Z

LGTM

vbatts · 2017-05-31T21:56:19Z

LGTM

Through f4d221c (Merge pull request opencontainers#880 from dqminh/wking-linux-only-capabilities-again, 2017-07-05). The rc6 release picked up an earlier version of these notes, and those entries are mostly unchanged except for: * The credentialSpec entry, which was opencontainers#814 for credentialspec and now also includes opencontainers#859 for credentialSpec. * The root(.path) Hyper-V entry, which was opencontainers#820 for root.path and now also includes opencontainers#838 for root. I also moved this into the "breaking changes" section, because rc5 Hyper-V configs required root to be set, and rc6 Hyper-V configs require it to not be set. Although whether rc5 allowed Hyper-V configs at all is not clear to me. * Fixed indenting for the typo-fixes entry, as well as a number of more recent typo-fix PRs. Signed-off-by: W. Trevor King <wking@tremily.us>

wking mentioned this pull request May 25, 2017

config-linux: Drop redundant 'blkio' prefix from blockIO properties #860

Merged

wking reviewed May 25, 2017

View reviewed changes

lowenna force-pushed the camelCase branch 4 times, most recently from 7ca2e6c to 451856a Compare May 25, 2017 23:12

lowenna changed the title ~~Fix camelCase and Solaris incosistencies~~ WIndows: Fix camelCase inconsistencies May 25, 2017

lowenna changed the title ~~WIndows: Fix camelCase inconsistencies~~ Windows: Fix camelCase inconsistencies May 25, 2017

Windows: Fix camelCase

c6338b3

Signed-off-by: John Howard <jhoward@microsoft.com>

lowenna force-pushed the camelCase branch from 451856a to c6338b3 Compare May 25, 2017 23:22

mrunalp approved these changes May 31, 2017

View reviewed changes

vbatts merged commit 09404ad into opencontainers:master May 31, 2017

lowenna deleted the camelCase branch May 31, 2017 21:56

vbatts mentioned this pull request Jul 5, 2017

bump to v1.0.0-rc6 #882

Merged

wking mentioned this pull request Jul 10, 2017

ChangeLog: Document changes since c83b8c8 #888

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Windows: Fix camelCase inconsistencies #859

Windows: Fix camelCase inconsistencies #859

lowenna commented May 25, 2017 •

edited

Loading

wking left a comment

wking May 25, 2017

lowenna May 25, 2017

wking May 25, 2017 •

edited

Loading

lowenna May 25, 2017

lowenna May 25, 2017

wking May 25, 2017

epilatow May 26, 2017

wking May 25, 2017

lowenna May 25, 2017

anuthan May 26, 2017

epilatow May 26, 2017

lowenna commented May 25, 2017 •

edited

Loading

wking commented May 25, 2017 via email

lowenna commented May 25, 2017

TomSweeneyRedHat commented May 26, 2017

mrunalp commented May 31, 2017 •

edited by caniszczyk

Loading

vbatts commented May 31, 2017 •

edited by caniszczyk

Loading

Windows: Fix camelCase inconsistencies #859

Windows: Fix camelCase inconsistencies #859

Conversation

lowenna commented May 25, 2017 • edited Loading

wking left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking May 25, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lowenna commented May 25, 2017 • edited Loading

wking commented May 25, 2017 via email

lowenna commented May 25, 2017

TomSweeneyRedHat commented May 26, 2017

mrunalp commented May 31, 2017 • edited by caniszczyk Loading

vbatts commented May 31, 2017 • edited by caniszczyk Loading

lowenna commented May 25, 2017 •

edited

Loading

wking May 25, 2017 •

edited

Loading

lowenna commented May 25, 2017 •

edited

Loading

mrunalp commented May 31, 2017 •

edited by caniszczyk

Loading

vbatts commented May 31, 2017 •

edited by caniszczyk

Loading