-
Notifications
You must be signed in to change notification settings - Fork 554
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
Adjust filesnames to match associated JSON #126
Conversation
The last commit (d83516e) adjust the lifecycle vs. master, but as I explain in that commit message, I don't think the lifecycle explained in master makes sense. If the explicit-destroy is unpalatable, we can use just “start” (for “create and start”) and “stop” (for “stop and destroy”) to get this PR landed while we work out the container/application separation (for more on this split, see this thread). I'm also personally in favor of allowing multiple applications per container, in which case I think we'll want something like wking/opencontainer-runtime-spec@4a87a44fa, but that's proved controversial in #107, so I'm leaving it out in the hopes that we can land this reorganization PR more quickly. |
- [Linux Specific Configuration](config-linux.md) | ||
- [Runtime and Lifecycle](runtime.md) | ||
- [Host-specific Container Configuration](runtime.md) | ||
- [Linux Specific Configuration](runtime-linux.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have two Linux Specific Configuration
one for the config-linux.md
and the other for runtime-linux.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- [Platform-independent Container Configuration](config.md)
- [Linux Specific Configuration](config-linux.md)
- [Host-specific Container Configuration](runtime.md)
- [Linux Host-specific Configuration](runtime-linux.md)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Sat, Aug 29, 2015 at 01:11:34AM -0700, Lai Jiangshan wrote:
you have two
Linux Specific Configuration
one for the
config-linux.md
and the other forruntime-linux.md
They share the same title because they are namespaced by the
higher-level entry. To be completely specific, it would be:
But that seemed too wordy.
Although I suppose I should at least convert to “Linux-specific” for
consistent hyphenation.
Before this commit we had the following mapping: * config.md → config.go (config.json) * config-linux.md → config_linux.go (config.json's "linux") * runtime-config.md → runtime_config.go (runtime.json) * runtime.md → lifecycle and runtime_config.go (runtime.json's "prestart" and "poststop" hooks) * runtime-config-linux.md → runtime_config_linux.go (runtime.json's "linux") This commit: * replaces underscores with hyphens, and: * renames Go and Markdown files so: * the docs for ${X}.json are in ${X}.md * the platform-independent types for ${X}.json are in ${X}.go * the docs for ${X}.json's ${PLATFORM} section are in ${X}-${PLATFORM}.md * the platform-dependent types for ${X}.json's ${PLATFORM} section are in ${X}-${PLATFORM}.go Signed-off-by: W. Trevor King <wking@tremily.us>
Hooks are only valid in runtime.json, so we should discuss them in the runtime.json docs. I'll add appropriate links from the lifecycle docs in a bit, but linking into runtime.json is poor enough that I think it deserves its own commit. The only change here besides the copy/paste is the addition of a blank line after the Markdown headers to match the rest of the documentation. Signed-off-by: W. Trevor King <wking@tremily.us>
Just like we link to the config.json docs. Signed-off-by: W. Trevor King <wking@tremily.us>
Just like we link to the config.json docs. Signed-off-by: W. Trevor King <wking@tremily.us>
Signed-off-by: W. Trevor King <wking@tremily.us>
It doesn't make sense to split creation into "create" and "start" while only having one "stop" command for both the application and container teardown.
To match 'Host-specific' and 'Host-independent'. Signed-off-by: W. Trevor King <wking@tremily.us>
To distinguish from the host-dependent runtime.json configuration. Signed-off-by: W. Trevor King <wking@tremily.us>
Signed-off-by: W. Trevor King <wking@tremily.us>
And add top-level headings for these files. Signed-off-by: W. Trevor King <wking@tremily.us>
This page is just about the lifecycle now. I'm not sure what the original intention was, but when this header landed in runtime.md with 5d2eb18 (*: re-org the spec, 2015-06-24) there wasn't anything beyond the lifecycle stuff in this file. Now that it's lifecycle.md and there are other files for our other content, I don't expect it to talk about anything besides the lifecycle in the future. Signed-off-by: W. Trevor King <wking@tremily.us>
da889ec
to
e22af5d
Compare
I don't think I'm doing anything controversial here, but this large renaming is going to be difficult to rebase as other PRs land in front of it (and possibly for folks writing other PRs if this lands in front of theirs, depending on whether their work touches something I did inside a file). It would be easiest if we could get an accept/reject on this one quickly to avoid needless rebasing work. If there's anything I can do to make this easier to review, or split off smaller chunks, let me know. |
I think it will be better if the pr is split into smaller ones. |
On Mon, Aug 31, 2015 at 05:53:57PM -0700, Lai Jiangshan wrote:
What do you think I can split off? I'm fine with folks merging / |
Your pr changes several things:
I think the PR can be split into these PRs. |
On Mon, Aug 31, 2015 at 06:51:34PM -0700, Lai Jiangshan wrote:
Which one of these are commits like 4cfa584? Once the rename makes I'd recommend folks just start in with the first PR, and then LGTM (or |
going to close this for a couple of things. Some of the linking is now outdated. Also, with trying to overhaul the approach to be more "top down" a lot of this will likely get changed further. |
On Mon, Oct 05, 2015 at 11:54:44AM -0700, Vincent Batts wrote:
Works for me, although I still think having runtime.md documenting |
And add more links to the runtime.json specs, since #88 didn't add
any. More details and motivation for each step in the commit
messages.