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

feat: support excluding digest tags #668

Merged
merged 7 commits into from
Nov 3, 2022

Conversation

qweeah
Copy link
Contributor

@qweeah qweeah commented Nov 2, 2022

Making this PR draft since I found that it's important to write good example and long description for the new flag.
Resolves #658
Signed-off-by: Billy Zha jinzha1@microsoft.com

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented Nov 2, 2022

Codecov Report

Merging #668 (68b1456) into main (ed9d584) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #668   +/-   ##
=======================================
  Coverage   72.12%   72.12%           
=======================================
  Files          14       14           
  Lines         513      513           
=======================================
  Hits          370      370           
  Misses        114      114           
  Partials       29       29           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@qweeah qweeah changed the title support excluding digest tags feat: support excluding digest tags Nov 2, 2022
Copy link
Member

@Wwwsylvia Wwwsylvia left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -52,6 +55,7 @@ Example - Show tags of the target repository that include values lexically after
},
}
cmd.Flags().StringVar(&opts.last, "last", "", "start after the tag specified by `last`")
cmd.Flags().BoolVar(&opts.excludeDigestTag, "exclude-digest-tag", false, "exclude all digest tags created by OCI artifact tag schema")
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe "referrers tag schema"?🤔

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 might need to rephrase this and add example latter, maybe without mentioning referrer or tag schema, below is why:

The use case of this flag is: one user found that oras repo tags is listing out some unexpected tags, he/she want to hide them. The struggling point is: how does this user know about the unwanted tags?

  • If he/she knows well of the distribution spec, then we only need to make the flag name explainative on the behavior of filtering or hidden
  • If he/she knows nothing of the distribution spec(most users don't), then mentioning referrers or tag schema in the long description won't help them realize that this flag is what they need to use.

Copy link
Member

Choose a reason for hiding this comment

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

I agree. The users don't need to know anything about the referrers implementation.

Copy link
Member

Choose a reason for hiding this comment

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

Brainstorming: How about "metadata tags"?

@qweeah qweeah marked this pull request as draft November 2, 2022 13:33
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah marked this pull request as ready for review November 3, 2022 04:22
cmd/oras/repository/show-tags.go Outdated Show resolved Hide resolved
cmd/oras/repository/show-tags.go Outdated Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah requested a review from shizhMSFT November 3, 2022 05:08
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

@shizhMSFT shizhMSFT merged commit 086edf7 into oras-project:main Nov 3, 2022
TerryHowe pushed a commit to TerryHowe/oras that referenced this pull request Feb 2, 2023
Making this PR draft since I found that it's important to write good
example and long description for the new flag.
Resolves oras-project#658
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
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.

support exclude digest tags in result of oras repo tags
5 participants