Skip to content
This repository was archived by the owner on May 15, 2024. It is now read-only.

Make subject optional #103

Merged
merged 2 commits into from
Jun 27, 2022
Merged

Make subject optional #103

merged 2 commits into from
Jun 27, 2022

Conversation

avtakkar
Copy link
Contributor

No description provided.

@avtakkar avtakkar requested a review from a team as a code owner June 15, 2022 00:46
Signed-off-by: Aviral Takkar <[email protected]>
@@ -28,7 +28,7 @@ type Manifest struct {

// Subject is an optional reference to any existing manifest within the repository.
// When specified, the artifact is said to be dependent upon the referenced subject.
Subject Descriptor `json:"subject"`
Subject Descriptor `json:"subject,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

How about this?

Suggested change
Subject Descriptor `json:"subject,omitempty"`
Subject *Descriptor `json:"subject,omitempty"`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated

Copy link
Contributor

@northtyphoon northtyphoon Jun 15, 2022

Choose a reason for hiding this comment

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

Do you consider to update other optional string properties in the spec?

Signed-off-by: Aviral Takkar <[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
Copy link
Contributor

/cc @michaelb990

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.

LGTM, thanks!

@sajayantony sajayantony merged commit 0b167f8 into oras-project:main Jun 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants