-
Notifications
You must be signed in to change notification settings - Fork 201
feat: support --format
for oras repo ls
#1758
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
Conversation
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1758 +/- ##
==========================================
+ Coverage 85.29% 85.39% +0.10%
==========================================
Files 133 137 +4
Lines 5882 5951 +69
==========================================
+ Hits 5017 5082 +65
- Misses 616 618 +2
- Partials 249 251 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to update the spec first? This implementation is not aligned to #733. E.g. if a sub namespace is used for MCR, the output is not repository but the output is saying it is
# E.g. `oras` is not a repository but `oss/v2/oras-project/oras` is
> oras repo ls mcr.microsoft.com/oss/v2/oras-project --format json
{
"repositories": [
"oras"
]
}
Nice catch, @qweeah ! I think we should have a refined model like $ oras repo ls mcr.microsoft.com/oss/v2/oras-project --format json
{
"registry": "mcr.microsoft.com",
"repositories": [
"oss/v2/oras-project/oras"
]
} so that it is consistent with the output of the distribution catalog API. |
SGTM |
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for a new --format
flag to oras repo ls
, enabling text
, json
, and go-template
outputs.
- Integrates
option.Format
into therepo ls
command and delegates output to new display handlers. - Updates command help/examples to document the flag and extends E2E tests for help, JSON, and Go-template formats.
- Introduces display packages (text, JSON, template, model, interface) and unit tests for handler initialization.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
test/e2e/suite/command/repo.go | Added E2E tests for --format help, JSON, and go-template outputs |
cmd/oras/root/repo/tags.go | Updated help examples to mention JSON and Go template formats |
cmd/oras/root/repo/ls.go | Integrated --format flag, display handlers, and updated examples |
cmd/oras/internal/display/metadata/text/repo_list.go | Added text format handler for repo ls |
cmd/oras/internal/display/metadata/template/repo_list.go | Added template format handler for repo ls |
cmd/oras/internal/display/metadata/model/repo_list.go | Added metadata model for repositories |
cmd/oras/internal/display/metadata/json/repo_list.go | Added JSON format handler for repo ls |
cmd/oras/internal/display/metadata/interface.go | Defined the RepoListHandler interface |
cmd/oras/internal/display/handler.go | Implemented the NewRepoListHandler factory |
cmd/oras/internal/display/handler_test.go | Added unit tests for NewRepoListHandler |
Comments suppressed due to low confidence (2)
cmd/oras/internal/display/handler_test.go:123
- [nitpick] Current tests only verify handler creation. Consider adding tests that call
OnRepositoryListed
andRender
for each format (text, JSON, template) to ensure actual output matches expectations.
handler, err := NewRepoListHandler(os.Stdout, tt.format, registry, namespace)
test/e2e/suite/command/repo.go:156
- The test calls
json.Unmarshal
but theencoding/json
package is not imported, causing a compile error. Please addimport "encoding/json"
to this file.
Expect(json.Unmarshal(bytes, &result)).ShouldNot(HaveOccurred())
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What this PR does / why we need it:
Introduces a new
--format
flag to theoras repo ls
command, with 3 format options:text
,json
, andgo-template
.--format text
: Equivalent to the default output, which remains unchanged--format json
: Outputs a JSON object withregistry
andrepositories
fields, where repositories include the full repository names--format go-template
: Uses the same underlying model as the JSON structureSpec: #1763
Introduced behaviors:



Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes part of #1547
Please check the following list: