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.

Sign up for GitHub

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

Jump to bottom

Working group updates #339

Merged
merged 1 commit into from
Sep 1, 2022
Merged

Working group updates #339

merged 1 commit into from
Sep 1, 2022

Conversation

sudo-bmitch
Copy link
Contributor

@sudo-bmitch sudo-bmitch commented Aug 26, 2022 •
edited
Loading

This is a follow up to #335 with changes noted in #337.

  • "refers" -> "refers field"
  • "referrers" -> "referrers list"
  • lower case for defined words in the spec
  • change reference to digest in the referrers API
  • add an error code for an invalid referrers API request
  • add a "note" to stress the risk of data loss

Signed-off-by: Brandon Mitchell git@bmitch.net

@michaelb990
Copy link
Contributor

Wow, thanks for putting this together so quickly. This is great. I worry about grouping all the nit / string changes with what looks like at least 2 functionality changes. Should we split off the bigger changes (SHOULD => MUST & maybe reference => digest) into separate issues to make sure we really have consensus and give folks who weren't at the meeting this morning a chance to chime in?

@sudo-bmitch
Working group updates
aa890a1 
- "refers" -> "refers field"
- "referrers" -> "referrers list"
- lower case for defined words in the spec
- change `reference` to `digest` in the referrers API
- add an error code for an invalid referrers API request
- add a "*note*" to stress the risk of data loss

Signed-off-by: Brandon Mitchell <git@bmitch.net>
@sudo-bmitch
Copy link
Contributor Author

@michaelb990 you got it. I left "reference => digest" because I think we intended that in our WG, but I removed the comment that we reserve the right to add tag support later while we get more consensus there. And the "SHOULD => MUST" change is also pulled from this PR.

@michaelb990 michaelb990 mentioned this pull request Aug 26, 2022
Closed
michaelb990
michaelb990 approved these changes Aug 26, 2022
View reviewed changes
Copy link
Contributor

@michaelb990 michaelb990 left a comment

Choose a reason for hiding this comment

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

IANAM but LGTM. Opened #340 to track the remaining question.

mikebrow
mikebrow approved these changes Aug 30, 2022
View reviewed changes
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.

LGTM

@sudo-bmitch sudo-bmitch merged commit a330bba into opencontainers:main Sep 1, 2022
@sudo-bmitch sudo-bmitch deleted the pr-wg-follow-up branch September 1, 2022 17:19
@jdolitsky jdolitsky mentioned this pull request Sep 15, 2022
Merged
@sudo-bmitch sudo-bmitch mentioned this pull request Feb 1, 2024
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikebrow mikebrow mikebrow approved these changes

@michaelb990 michaelb990 michaelb990 approved these changes

@jdolitsky jdolitsky jdolitsky approved these changes

@dmcgowan dmcgowan Awaiting requested review from dmcgowan dmcgowan is a code owner

@jzelinskie jzelinskie Awaiting requested review from jzelinskie jzelinskie is a code owner

@jonjohnsonjr jonjohnsonjr Awaiting requested review from jonjohnsonjr jonjohnsonjr is a code owner

@stevvooe stevvooe Awaiting requested review from stevvooe stevvooe is a code owner

@vbatts vbatts Awaiting requested review from vbatts

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sudo-bmitch @michaelb990 @jdolitsky @mikebrow