-
Notifications
You must be signed in to change notification settings - Fork 55
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
Empty array stdout with --json flag #747
Conversation
@@ -1,49 +0,0 @@ | |||
config: |
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.
File was rolled-into the rules test cases file.
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.
Nice improvement.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #747 +/- ##
==========================================
- Coverage 71.67% 71.56% -0.12%
==========================================
Files 89 89
Lines 11160 11207 +47
==========================================
+ Hits 7999 8020 +21
- Misses 2658 2682 +24
- Partials 503 505 +2
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
@@ -149,12 +149,15 @@ func (r *Renderer) LogList(logs []*management.Log, silent, hasFilter bool) { | |||
|
|||
if len(logs) == 0 { |
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.
Suggestion:
if len(logs) == 0 {
if !hasFilter {
r.EmptyState(resource, "To generate logs, run a test command like 'auth0 test login' or 'auth0 test token'")
return
}
r.EmptyState("logs matching filter criteria", "")
return
}
Like this it will print "No logs matching filter criteria available." and we have less complex code.
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.
IMO that reads very poorly. I'm glad we were able to reduce a couple one-off instances, but I think this one still makes sense.
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.
Both sentences are grammatically correct and convey the same meaning 🤷🏻♂️ Not a blocker tho.
Co-authored-by: Sergiu Ghitea <[email protected]>
Co-authored-by: Sergiu Ghitea <[email protected]>
…0-cli into empty-array-json-flag
🔧 Changes
Previously, #736 was introduced to return an empty array when no results and the
--json
flag was passed. This addressed several commands but still missed some commands. There are certainly opportunities to refactor the code and abstract these empty state handlers but I mostly just want to get some implementation in and add test cases.📚 References
Initial PR: #736
🔬 Testing
Added integration test cases wherever practically possible. Had to reorganize the test files to make them deterministic and independent from each other.
📝 Checklist