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

Clarify specification on Bundle and Envelope media types #283

Merged
merged 6 commits into from
Oct 10, 2023

Conversation

marcelamelara
Copy link
Contributor

@marcelamelara marcelamelara commented Sep 19, 2023

This PR updates the specs for Bundles and Envelopes to more formally describe conventions around attestation layer media types when these are stored in arbitrary storage systems, per #271 (comment)

Fixes #271

@marcelamelara marcelamelara requested a review from a team as a code owner September 19, 2023 00:05
Copy link
Contributor

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

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

I agree that this PR improves clarity, thanks for working on it.

I'm worried we're not consistent in using capitalised names for components in the attestation model (Bundle, Attestation, Envelope, Statement, Predicate, etc). Should we do a consistency pass and create some contributor documentation explaining conventions like this?

spec/v1/bundle.md Outdated Show resolved Hide resolved
spec/v1/bundle.md Outdated Show resolved Hide resolved
spec/v1/envelope.md Outdated Show resolved Hide resolved
@marcelamelara
Copy link
Contributor Author

I'm worried we're not consistent in using capitalised names for components in the attestation model (Bundle, Attestation, Envelope, Statement, Predicate, etc). Should we do a consistency pass and create some contributor documentation explaining conventions like this?

Agreed, I don't think we're being consistent on this right now. Personally, I find it helpful to capitalize these terms denote when we're talking about attestation model components vs. not, but I wasn't sure if everyone would agree. If we do, then we should definitely follow your suggestion of doing a consistency pass and adding documentation for this.

spec/v1/bundle.md Outdated Show resolved Hide resolved
spec/v1/bundle.md Show resolved Hide resolved
spec/v1/envelope.md Outdated Show resolved Hide resolved
spec/v1/envelope.md Show resolved Hide resolved
spec/v1/envelope.md Show resolved Hide resolved
spec/v1/envelope.md Outdated Show resolved Hide resolved
spec/v1/envelope.md Show resolved Hide resolved
spec/v1/bundle.md Outdated Show resolved Hide resolved
spec/v1/bundle.md Outdated Show resolved Hide resolved
spec/v1/envelope.md Outdated Show resolved Hide resolved
spec/v1/envelope.md Outdated Show resolved Hide resolved
spec/v1/envelope.md Outdated Show resolved Hide resolved
spec/v1/bundle.md Outdated Show resolved Hide resolved
spec/v1/envelope.md Outdated Show resolved Hide resolved
spec/v1/bundle.md Outdated Show resolved Hide resolved
Copy link
Member

@adityasaky adityasaky left a comment

Choose a reason for hiding this comment

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

Some minor comments / updates aside, this LGTM.

spec/v1/envelope.md Outdated Show resolved Hide resolved
spec/v1/envelope.md Show resolved Hide resolved
spec/v1/envelope.md Outdated Show resolved Hide resolved
Copy link
Contributor

@TomHennen TomHennen left a comment

Choose a reason for hiding this comment

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

This looks great!

spec/v1/bundle.md Outdated Show resolved Hide resolved
spec/v1/bundle.md Outdated Show resolved Hide resolved
@marcelamelara marcelamelara force-pushed the clarify-layers-media-type branch 2 times, most recently from 6338c91 to 54ad0ae Compare October 6, 2023 23:53
Copy link
Member

@trishankatdatadog trishankatdatadog left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

Copy link
Contributor

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

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

This is excellent, thank you!

spec/v1/bundle.md Outdated Show resolved Hide resolved
spec/v1/envelope.md Outdated Show resolved Hide resolved
spec/v1/bundle.md Outdated Show resolved Hide resolved
@TomHennen TomHennen merged commit 09a0315 into in-toto:main Oct 10, 2023
1 check passed
@marcelamelara marcelamelara deleted the clarify-layers-media-type branch August 30, 2024 19:16
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.

Clearly document the mediaType of in-toto attestation layers
5 participants