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

spec: consistent wording #641

Merged
merged 1 commit into from
Apr 7, 2017
Merged

Conversation

jonboulle
Copy link
Contributor

Chasing #621, standardise on case/punctuation/articles

Signed-off-by: Jonathan Boulle jonathanboulle@gmail.com

@vbatts
Copy link
Member

vbatts commented Apr 6, 2017

fair
LGTM

Approved with PullApprove

spec.md Outdated
* [Image Index](image-index.md) - An index of annotated image manifests distributed as a complete unit.
* [Image Layout](image-layout.md) - The filesystem layout representing the contents of an image
* [Image Manifest](manifest.md) - a document describing the components that make up a container image
* [Image Index](image-index.md) - an index of annotated image manifests distributed as a complete unit
Copy link
Contributor

Choose a reason for hiding this comment

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

“an index of annotated image manifests” → “an annotated index of image manifests”? The index has annotated descriptors pointing at image manifests; whether the manifests themselves contain annotations doesn't really matter here.

And I'd rather drop “complete unit”, since the presence of an index is really about indexing multiple things, and not about whether the DAGs referenced from the index are present in the same CAS engine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup

spec.md Outdated
* [Filesystem Layers](layer.md) - changesets that describe a container's filesystem
* [Image Configuration](config.md) - The document determining layer ordering and configuration of the image suitable for consumption by runtime-spec
* [Descriptors](descriptor.md) - Describes the type, metadata and content address of referenced content.
* [Image Configuration](config.md) - a document determining layer ordering and configuration of the image suitable for consumption by [runtime-spec][]
Copy link
Contributor

Choose a reason for hiding this comment

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

runtime-spec isn't really the consumer (that would probably be the runtime implementation). How about:

… suitable for translation into a [runtime bundle][…].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point

spec.md Outdated
* [Image Configuration](config.md) - The document determining layer ordering and configuration of the image suitable for consumption by runtime-spec
* [Descriptors](descriptor.md) - Describes the type, metadata and content address of referenced content.
* [Image Configuration](config.md) - a document determining layer ordering and configuration of the image suitable for consumption by [runtime-spec][]
* [Descriptors](descriptor.md) - an reference that describes the type, metadata and content address of referenced content
Copy link
Contributor

Choose a reason for hiding this comment

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

Everything else in this list is singular. How about:

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think "an reference" is correct english, is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's not, typo

Chasing opencontainers#621, standardise on case/punctuation/articles

Signed-off-by: Jonathan Boulle <jonathanboulle@gmail.com>
@jonboulle
Copy link
Contributor Author

thanks for the suggestions, should be good to go now

@vbatts
Copy link
Member

vbatts commented Apr 7, 2017

LGTM

Approved with PullApprove

* [Image Configuration](config.md) - The document determining layer ordering and configuration of the image suitable for consumption by runtime-spec
* [Descriptors](descriptor.md) - Describes the type, metadata and content address of referenced content.
* [Image Manifest](manifest.md) - a document describing the components that make up a container image
* [Image Index](image-index.md) - an annotated index of one or more image manifests
Copy link
Contributor

Choose a reason for hiding this comment

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

“one or more” is not accurate; we explicitly allow manifests to be empty. This allows you to do things like distribute an image-layout which only provides CAS blobs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, “image manifests” is overly specific (see here and here), but I'm fine glossing over that for this entry.

@stevvooe
Copy link
Contributor

stevvooe commented Apr 7, 2017

LGTM

Approved with PullApprove

@stevvooe stevvooe merged commit a90602e into opencontainers:master Apr 7, 2017
wking added a commit to wking/image-spec that referenced this pull request Apr 7, 2017
Fixing overly-strict wording from 0cc871e (spec: consistent wording,
opencontainers#641).  In image-index.md, we explicitly allow 'manifests' to be
empty:

  While this property MUST be present, the size of the array MAY be
  zero.

Which allows you to do things like distribute an image-layout which
only provides CAS blobs.

Signed-off-by: W. Trevor King <wking@tremily.us>
@vbatts vbatts mentioned this pull request May 19, 2017
jonboulle pushed a commit to jonboulle/image-spec that referenced this pull request May 22, 2017
Fixing overly-strict wording from 0cc871e (spec: consistent wording,
opencontainers#641).  In image-index.md, we explicitly allow 'manifests' to be
empty:

  While this property MUST be present, the size of the array MAY be
  zero.

Which allows you to do things like distribute an image-layout which
only provides CAS blobs.

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.

5 participants