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

Clean-up spec to remove non-spec-ish items #458

Merged
merged 1 commit into from Dec 1, 2016

Conversation

duglin
Copy link
Collaborator

@duglin duglin commented Nov 16, 2016

Per the oci f2f at kubecon. Similar changes what is in: opencontainers/runtime-spec#626

Signed-off-by: Doug Davis dug@us.ibm.com

- [Notational Conventions](#notational-conventions)
- [Overview](#overview)
- [Understanding the Specification](#understanding-the-specification)
- [Media Types](media-types.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

Inconsistent indent? You should replace the earlier tab with four spaces (opencontainers/runtime-spec#495).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@wking
Copy link
Contributor

wking commented Nov 16, 2016

I think this is a step in the right direction, although it will
conflict with a few open PRs against this language (#455, #456).

I also think we should merge the “Overview” and “Understanding the
Specification” sections 1, but that can happen in follow-up work if
you don't want to address it here. Pulling out “Notational
Conventions”, like you've done here, will make future merging easier.

@philips
Copy link
Contributor

philips commented Nov 17, 2016

LGTM

Approved with PullApprove

@@ -118,4 +118,4 @@ Instead they MUST ignore unknown properties.
}
```

[runtime-platform]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc2/config.md#platform
[runtime-platform2]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc2/config.md#platform
Copy link
Contributor

Choose a reason for hiding this comment

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

Why rename this?

Copy link
Contributor

Choose a reason for hiding this comment

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

The current approach to single-file PDF/HTML generation is to feed all the Markdown into Pandoc. Pandoc seems to treat them as a single, concatenated file internally and doesn't like seeing the same reference defined more than once, so he's giving this a globally-unique name to avoid conflicting with the entry in config.md.

Copy link
Contributor

Choose a reason for hiding this comment

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

Gah, what a mess. I really just want to drop PDF/HTML.

@vbatts
Copy link
Member

vbatts commented Nov 30, 2016

@duglin rebase please?

@duglin
Copy link
Collaborator Author

duglin commented Nov 30, 2016

rebased, but I'd appreciate a double-check to make sure I didn't drop anything from README.md or spec.md.

@jonboulle
Copy link
Contributor

Conflicts again

Signed-off-by: Doug Davis <dug@us.ibm.com>
@duglin
Copy link
Collaborator Author

duglin commented Dec 1, 2016

rebased

@jonboulle
Copy link
Contributor

jonboulle commented Dec 1, 2016

lgtm I suppose

Approved with PullApprove

@vbatts
Copy link
Member

vbatts commented Dec 1, 2016

LGTM

Approved with PullApprove

@vbatts vbatts merged commit 241dd68 into opencontainers:master Dec 1, 2016
wking added a commit to wking/image-spec that referenced this pull request Dec 10, 2016
This landed in 836fb1c ([ReadMe] Add Compliance Language, 2016-10-11,
opencontainers#432) but was removed in 27508e2 (Clean-up spec to remove
non-spec-ish items, 2016-11-14, opencontainers#458), likely accidentally during a
rebase.

Signed-off-by: W. Trevor King <wking@tremily.us>
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.

None yet

5 participants