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

add concept for OCI Artifact #238

Merged
merged 12 commits into from
Aug 9, 2023
Merged

Conversation

sajayantony
Copy link
Contributor

@sajayantony sajayantony commented Aug 4, 2023

Signed-off-by: Sajay Antony <[email protected]>
SteveLasker
SteveLasker previously approved these changes Aug 4, 2023
Copy link
Contributor

@SteveLasker SteveLasker left a comment

Choose a reason for hiding this comment

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

Overall, looks great with feedback.
Biggest concern is the example for multiple SBOMs in an index.
Minor nit/question. What is the formatting guidance? This PR uses the 80 character rule, as opposed to sentences on separate lines for easier commenting.

docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Show resolved Hide resolved
docs/concepts/artifact.mdx Show resolved Hide resolved
docs/concepts/artifact.mdx Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Show resolved Hide resolved
Co-authored-by: Shiwei Zhang <[email protected]>
Signed-off-by: Sajay Antony <[email protected]>
@sajayantony
Copy link
Contributor Author

Good question. I generally preffer the 80 character limit since its easier to read without rendering, but can change it whatever the guidance /cc @FeynmanZhou is this called out somewhere?

Signed-off-by: Sajay Antony <[email protected]>
@TerryHowe
Copy link
Member

What is the formatting guidance? This PR uses the 80 character rule, as opposed to sentences on separate lines for easier commenting.

I don't know if we have an official standard, but I prefer https://sembr.org/

Signed-off-by: Sajay Antony <[email protected]>
@sajayantony
Copy link
Contributor Author

@FeynmanZhou @toddysm @shizhMSFT PTAL.

docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/tag_relations.dot Outdated Show resolved Hide resolved
@SteveLasker
Copy link
Contributor

Good question. I generally preffer the 80 character limit since its easier to read without rendering, but can change it whatever the guidance /cc @FeynmanZhou is this called out somewhere?

The benefit of using separate lines for each sentence is you can scope a comment, make a PR to a specific line.
I suppose 80 characters and 1 sentence per line aren't mutually exclusive, but we should adopt these to simplify reading/commenting and subsequent PRs.

Signed-off-by: Sajay Antony <[email protected]>
Signed-off-by: Sajay Antony <[email protected]>
Signed-off-by: Sajay Antony <[email protected]>
docs/concepts/tag_relations.dot Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
docs/concepts/artifact.mdx Outdated Show resolved Hide resolved
sajayantony and others added 3 commits August 9, 2023 00:44
Co-authored-by: Shiwei Zhang <[email protected]>
Signed-off-by: Sajay Antony <[email protected]>
Co-authored-by: Shiwei Zhang <[email protected]>
Signed-off-by: Sajay Antony <[email protected]>
Co-authored-by: Shiwei Zhang <[email protected]>
Signed-off-by: Sajay Antony <[email protected]>
Signed-off-by: Sajay Antony <[email protected]>
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@sajayantony sajayantony merged commit cf5a252 into oras-project:main Aug 9, 2023
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.

6 participants