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

Remove references to artifact manifest and artifact manfiest #407

Merged
merged 2 commits into from
May 4, 2023

Conversation

brackendawson
Copy link
Contributor

No description provided.

jdolitsky
jdolitsky previously approved these changes May 3, 2023
sudo-bmitch
sudo-bmitch previously approved these changes May 3, 2023
spec.md Outdated Show resolved Hide resolved
spec.md Outdated Show resolved Hide resolved
@@ -480,7 +480,7 @@ Client and registry implementations SHOULD expect to be able to support manifest

###### Pushing Manifests with Subject

When processing a request for an image or artifact manfiest with the `subject` field, a registry implementation that supports the [referrers API](#listing-referrers) MUST respond with the response header `OCI-Subject: <subject digest>` to indicate to the client that the registry processed the request's `subject`.
When processing a request for an image manifest with the `subject` field, a registry implementation that supports the [referrers API](#listing-referrers) MUST respond with the response header `OCI-Subject: <subject digest>` to indicate to the client that the registry processed the request's `subject`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
When processing a request for an image manifest with the `subject` field, a registry implementation that supports the [referrers API](#listing-referrers) MUST respond with the response header `OCI-Subject: <subject digest>` to indicate to the client that the registry processed the request's `subject`.
When processing a request for a manifest with the `subject` field, a registry implementation that supports the [referrers API](#listing-referrers) MUST respond with the response header `OCI-Subject: <subject digest>` to indicate to the client that the registry processed the request's `subject`.

Copy link
Contributor

Choose a reason for hiding this comment

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

We haven't approved the subject field on the index manifest yet.

Copy link
Member

Choose a reason for hiding this comment

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

ok.. either way is there benefit to restricting the MUST to image manifest... noting the section title is the broader Manifests

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't that require registries to parse manifests for a field that may not have been included in the image-spec?

Copy link
Member

Choose a reason for hiding this comment

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

yes I believe so.. Though my generalization was more to reduce complexity in the wording here for when/if subject is added to index... I didn't contemplate that this push MUST was an implied but only for genuine oci.image.manifest.v1+json manifests with said subject field (implying as of v1.1.0).. hmm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm with Brandon on this specific one, it would introduces another ambiguity. Also bordering on scope creep here, my aim was to not say "artifact manifest in the document", the previous version did explicitly list the manifests it applied to.

Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

see comments

@brackendawson brackendawson dismissed stale reviews from sudo-bmitch and jdolitsky via 8e2a64e May 4, 2023 10:41
@jdolitsky jdolitsky merged commit 57b69b6 into opencontainers:main May 4, 2023
@brackendawson brackendawson deleted the aahtifacts branch May 4, 2023 16:17
@jdolitsky jdolitsky mentioned this pull request Jun 27, 2023
8 tasks
@jdolitsky jdolitsky mentioned this pull request Jul 6, 2023
8 tasks
@sudo-bmitch sudo-bmitch mentioned this pull request Feb 1, 2024
8 tasks
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.

4 participants