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

README: rework description of spec components #621

Merged
merged 1 commit into from
Apr 5, 2017

Conversation

erikh
Copy link
Contributor

@erikh erikh commented Mar 21, 2017

This replaces the inline links with a bulleted list that clearly states each part of the spec and a little blurb that describes it. I think it's a much easier way to navigate, I hope you agree.

I also made the README link to the spec a lot more prominent.

@vbatts
Copy link
Member

vbatts commented Mar 21, 2017

@RobDolinMS is this PR what you were referring to?

@jbouzane
Copy link

I'm concerned about this PR because it presents itself as a formatting change, but it's removing some text from spec.md. Is that intentional, or did it just get lost in the change?

@erikh
Copy link
Contributor Author

erikh commented Mar 23, 2017 via email

@stevvooe
Copy link
Contributor

stevvooe commented Apr 3, 2017

I'm concerned about this PR because it presents itself as a formatting change, but it's removing some text from spec.md. Is that intentional, or did it just get lost in the change?

These changes represent a more realistic presentation of the specification.

It looks like there is some language here that was removed for road map items (signatures and naming/federation).

@erikh Would you mind adding those bullets back?

@erikh
Copy link
Contributor Author

erikh commented Apr 4, 2017 via email

@erikh
Copy link
Contributor Author

erikh commented Apr 4, 2017

@stevvooe I've made the updates, is this what you're after?

Copy link
Contributor

@jonboulle jonboulle left a comment

Choose a reason for hiding this comment

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

I appreciate the effort, but frankly I find all these short descriptions a bit confusing. Do we need them here? If so, can we just re-use the tagline or high level description from the respective documents?

spec.md Outdated
* An archival format for container images, consisting of an [image manifest](manifest.md), an [image index](image-index.md) (optional), an [image layout](image-layout.md), a set of [filesystem layers](layer.md), and [image configuration](config.md) (base OCI layer)
* A [process of referencing container images by a cryptographic hash of their content](descriptor.md) (base OCI layer)
* A format for [storing CAS blobs and references to them](image-layout.md) (optional OCI layer)
* [Image Manifest](manifest.md) - the stored document describing an image's 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 is the "stored document" stored?

spec.md Outdated
* A [process of referencing container images by a cryptographic hash of their content](descriptor.md) (base OCI layer)
* A format for [storing CAS blobs and references to them](image-layout.md) (optional OCI layer)
* [Image Manifest](manifest.md) - the stored document describing an image's properties
* [Image Index](image-index.md) - the stored document describing a collection of images redistributed as one
Copy link
Contributor

Choose a reason for hiding this comment

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

Is an image index really a collection of images redistributed as one? (It sort of implies it's bundled together, is that necessarily the case?)

Copy link
Contributor

Choose a reason for hiding this comment

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

More or less. Effectively, they reference one or more manifests, annotating the underlying content with platform information "pulled up" config. In practice, we could compare this to a "fat binary", although the content isn't embedded.

"An index of annotated image manifests distributed as a complete unit."

Not great, but that might help to better describe this.

spec.md Outdated
* [Image Manifest](manifest.md) - the stored document describing an image's properties
* [Image Index](image-index.md) - the stored document describing a collection of images redistributed as one
* [Image Layout](image-layout.md) - the filesystem layout representing the contents of an image
* [Filesystem Layers](layer.md) - the building block of image data
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO this description is a bit vague

Copy link
Contributor

Choose a reason for hiding this comment

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

"changesets that describe a container's filesystem"

spec.md Outdated
* [Image Layout](image-layout.md) - the filesystem layout representing the contents of an image
* [Filesystem Layers](layer.md) - the building block of image data
* [Image Configuration](config.md) - the stored document determining layer ordering and configuration of the image suitable for consumption by runtime-spec
* [Descriptors](descriptor.md) - a concept used to refer to container images by the hash of their content
Copy link
Contributor

Choose a reason for hiding this comment

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

it's not just container images, it's all components of the spec

Copy link
Contributor

Choose a reason for hiding this comment

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

"Describes the type, metadata and content address of referenced content."

spec.md Outdated
* [Image Index](image-index.md) - the stored document describing a collection of images redistributed as one
* [Image Layout](image-layout.md) - the filesystem layout representing the contents of an image
* [Filesystem Layers](layer.md) - the building block of image data
* [Image Configuration](config.md) - the stored 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.

Not really suitable for consumption; maybe this would more appropriately link to what comes out of #492

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do we do for now though?

@jonboulle jonboulle changed the title Rewrite spec links README: rework description of spec components Apr 4, 2017
@erikh
Copy link
Contributor Author

erikh commented Apr 4, 2017 via email

@stevvooe
Copy link
Contributor

stevvooe commented Apr 4, 2017

@stevvooe I've made the updates, is this what you're after?

Yea, the separation looks good.

The descriptions of each component need a little work. I'll comment in line.

spec.md Outdated
* An archival format for container images, consisting of an [image manifest](manifest.md), an [image index](image-index.md) (optional), an [image layout](image-layout.md), a set of [filesystem layers](layer.md), and [image configuration](config.md) (base OCI layer)
* A [process of referencing container images by a cryptographic hash of their content](descriptor.md) (base OCI layer)
* A format for [storing CAS blobs and references to them](image-layout.md) (optional OCI layer)
* [Image Manifest](manifest.md) - the stored document describing an image's properties
Copy link
Contributor

Choose a reason for hiding this comment

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

"Describes the components that make up a container image."

@jonboulle
Copy link
Contributor

jonboulle commented Apr 4, 2017 via email

@erikh
Copy link
Contributor Author

erikh commented Apr 5, 2017

Thanks for the guidance, should have these incorporated by EOD. I know the schedule is tight.

Signed-off-by: Erik Hollensbe <github@hollensbe.org>
@erikh
Copy link
Contributor Author

erikh commented Apr 5, 2017

PTAL

@vbatts
Copy link
Member

vbatts commented Apr 5, 2017

LGTM

Approved with PullApprove

1 similar comment
@stevvooe
Copy link
Contributor

stevvooe commented Apr 5, 2017

LGTM

Approved with PullApprove

@stevvooe stevvooe merged commit e9f698e into opencontainers:master Apr 5, 2017
@erikh erikh deleted the rewrite-spec-links branch April 6, 2017 09:29
jonboulle added a commit to jonboulle/image-spec that referenced this pull request Apr 6, 2017
Chasing opencontainers#621, standardise on case/punctuation/articles

Signed-off-by: Jonathan Boulle <jonathanboulle@gmail.com>
jonboulle added a commit to jonboulle/image-spec that referenced this pull request Apr 7, 2017
Chasing opencontainers#621, standardise on case/punctuation/articles

Signed-off-by: Jonathan Boulle <jonathanboulle@gmail.com>
@vbatts vbatts mentioned this pull request May 19, 2017
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