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

Add references to Windows containers #554

Closed
wants to merge 2 commits into from

Conversation

RobDolinMS
Copy link
Collaborator

Signed-off-by: Rob Dolin [email protected]

@@ -0,0 +1,3 @@
# Windows Application Container Configuration

Windows application containers can be configured using the following properties.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where's the meat? Is this supposed to have stuff from #504?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wking To your question of "Where's the beef", there's not much here yet. I would expect that #504 and similar would be added over time. My desire with this PR it clear that it is the intent of OCI to support Linux, Solaris, and Windows.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Thu, Sep 01, 2016 at 04:11:39PM -0700, Rob Dolin (MSFT) wrote:

@@ -0,0 +1,3 @@
+# Windows Application Container Configuration
+
+Windows application containers can be configured using the following properties.

… My desire with this PR it clear that it is the intent of OCI to
support Linux, Solaris, and Windows.

Then how about dropping this file (until you have Windows-specific
settings to put into it) and using:

in README.md?

@RobDolinMS
Copy link
Collaborator Author

Signed-off-by: Rob Dolin [email protected]

@RobDolinMS
Copy link
Collaborator Author

@opencontainers/runtime-spec-maintainers Per conversation on OCI Dev ConCall, please LGTM and merge. Thanks. :)

@wking
Copy link
Contributor

wking commented Sep 7, 2016

On Wed, Sep 07, 2016 at 02:19:30PM -0700, Rob Dolin (MSFT) wrote:

Signed-off-by: Rob Dolin [email protected]

Ah, looks like the Signed-off-by in d407660 is missing a trailing
‘>’. Can you amend and force-push?

@RobDolinMS
Copy link
Collaborator Author

Doh. Thanks @wking. I see my typo at the top of: d407660

With the tool chain I'm using, I can't easily amend and force-push. :(

If two maintainer LGTMs cannot merge this change, please LMK and I'll re-create the PR and re-submit.

@tianon
Copy link
Member

tianon commented Sep 8, 2016

@RobDolinMS yeah, I think the DCO check is a firm blocker (AIUI, IANAL); if your current toolchain can't amend/force, could you clone your branch elsewhere and amend/force there? Otherwise I think you're stuck with a resubmit. 😞

@wking
Copy link
Contributor

wking commented Sep 8, 2016

On Thu, Sep 08, 2016 at 01:50:48PM -0700, Tianon Gravi wrote:

@RobDolinMS yeah, I think the DCO check is a firm blocker (AIUI,
IANAL); if your current toolchain can't amend/force, could you clone
your branch elsewhere and amend/force there? Otherwise I think
you're stuck with a resubmit. 😞

I believe this has been dealt with in the past by having the merging
maintainer insert Rob's Signed-off-by in the commit message when
merging (in cases where he has provided that Signed-off-by via a
GitHub comment). But I haven't dug up a specific instance, so I may
be miss-remembering.

@RobDolinMS
Copy link
Collaborator Author

Working on a replacement for this with more meat, and meeting with Windows Containers experts later today

@RobDolinMS
Copy link
Collaborator Author

Closing per above comments.

@RobDolinMS RobDolinMS closed this Sep 14, 2016
@RobDolinMS RobDolinMS deleted the patch-10 branch September 14, 2016 23:16
@wking
Copy link
Contributor

wking commented Sep 14, 2016

On Wed, Sep 14, 2016 at 04:16:45PM -0700, Rob Dolin (MSFT) wrote:

Closing per above comments.

With the Windows changes coming in in a stream of tiny PRs (which I
like for focused review ;), somebody is still going to have to land
the README protocol reference you have here 1. I suggest re-opening
this PR to land just that line, and have whoever lands the first meat
in a config-windows.md take care of the rest of what you have here
now.

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.

3 participants