-
Notifications
You must be signed in to change notification settings - Fork 97
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
Fix rad group show
and rad group list
#6252
Conversation
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Test Results2 984 tests ±0 2 975 ✔️ ±0 2m 19s ⏱️ +19s Results for commit 42801c2. ± Comparison against base commit 33c51ed. This pull request removes 2 and adds 2 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
Heading: "Name", | ||
JSONPath: "{ .name }", | ||
Heading: "NAME", | ||
JSONPath: "{ .Name }", |
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.
I'm curious about why this fixes the issue. If this is based on JSON then the previous code was correct:
func (r ResourceGroupResource) MarshalJSON() ([]byte, error) { |
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 you want to delay merging this until we figure it out? Or can we merge this in to fix the bug?
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.
I fired this up in the debugger and understood what's going on.
My expectation was that the JSONPointer library would operate on the JSON representation of the data. It turns out that it doesn't. It operates on the Go structs. So the behavior and the fix makes sense, it's not just not what I would have designed 😆
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
Description
This PR updates the JSONPath mappings for resource groups. Fixes a regression introduced by #6191
Type of change
Fixes: #6249
Auto-generated summary
🤖 Generated by Copilot at 47c16a4
Summary
🐛🧪📦
Added a CLI test for listing resource groups and fixed JSONPath expressions for the output format.
Walkthrough
ResourceGroupResource
struct in thetable
format options (link)objectformats
andto
packages to use theGetResourceGroupTableFormat
function and theto.Ptr
helper function in thegroup list
command test (link)ResourceGroupResource
objects in thegroup list
command test (link)FormattedOutput
objects with thetable
format and theGetResourceGroupTableFormat
options in thegroup list
command test (link)outputSink
in thegroup list
command test (link)