Skip to content
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

config.md: Remove 'solaris' from full example #436

Merged

Conversation

wking
Copy link
Contributor

@wking wking commented May 16, 2016

This should have been part of 759ee79 (config: Add platform-specific entry for 'solaris', 2016-05-06, #431), since the example has platform.os set to linux.

There was some (brief) discussion of this point before the solaris section landed, but the “should only be set if” wording landed in parallel via b373a15 (config: Split platform-specific configuration into its own section, 2016-05-02, #414), and I'd forgotten to go back and apply that logic to #411.

Having a full Solaris example would be useful, but I think it should be a separate, Solaris-only example.

This should have been part of 759ee79 (config: Add
platform-specific entry for 'solaris', 2016-05-06, opencontainers#431), since
the example has platform.os set to 'linux'.

There was some (brief) discussion of this point before the 'solaris'
section landed [1], but the "should only be set if" wording landed in
parallel via b373a15 (config: Split platform-specific configuration
into its own section, 2016-05-02, opencontainers#414), and I'd forgotten to go back
and apply that logic to opencontainers#411.

Having a full Solaris example would be useful, but I think it should
be a separate, Solaris-only example.

[1]: opencontainers#411 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking wking force-pushed the remove-solaris-from-full-config branch from 17f77d4 to a044e07 Compare May 16, 2016 06:06
@mrunalp
Copy link
Contributor

mrunalp commented May 18, 2016

Yeah, it makes sense to create a different solaris example.

@wking
Copy link
Contributor Author

wking commented May 18, 2016

On Wed, May 18, 2016 at 07:12:13AM -0700, Mrunal Patel wrote:

Yeah, it makes sense to create a different solaris example.

Should that happen in this PR? I'm not familiar enough with Solaris
to know what reasonable values are for the rest of the config, but it
would be nice if we were copying the Solaris-specific block somewhere
new instead of deleting it here and then restoring it later.

@crosbymichael
Copy link
Member

LGTM

1 similar comment
@mrunalp
Copy link
Contributor

mrunalp commented May 18, 2016

LGTM

@mrunalp mrunalp merged commit 7cdb70f into opencontainers:master May 18, 2016
@anuthan
Copy link
Contributor

anuthan commented May 18, 2016

This is fine, I will work on the Solaris only example in the coming days.

@wking wking deleted the remove-solaris-from-full-config branch May 20, 2016 04:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants