-
Notifications
You must be signed in to change notification settings - Fork 370
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
Add format flag to listen cmd #615
Conversation
The format flag is a string that currently only takes in JSON as a valid input option. It's inteded to replace the --print-json for the listen cmd. The difference in behavior between --format JSON and --print-json is that the new flag will stripe out all formatting in the payload.
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 have some suggestions for the output strings 😄
Oh my goodness, thank you for catching these! |
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.
The messages look good to me. I can't speak for the rest of it so passing back r? @tomer-stripe
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.
This looks good! Two quick questions:
- is
JSON
case sensitive? It probably shouldn't be but I can't remember what we do in request logs - did you test this with piping into
jq
? We should make sure that still works (and potentially mention that in the deprecation message too)
LGTM but same questions as @tomer-stripe for case sensitivity. As a user I think I'd default to |
|
Okay, I updated the flag check to be case insensitive for both And the updated deprecation message: ptal @tomer-stripe |
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.
LOVE IT!
Reviewers
r? @tomer-stripe
cc @stripe/developer-products
Summary
The --format flag is a string that currently only takes in JSON as a valid input option, similar to the log tailing command.
It's intended to replace the --print-json for the listen cmd. The difference in behavior between --format JSON and --print-json is that the new flag will stripe out all formatting in the payload.
While working on the event streaming feature in VSCode, the stream was getting partial json because the json was being pretty-printed. This PR updates the listen cmd to behave the same way as the log tail command.
Testing
With the deprecated flag:
![Screen Shot 2021-03-17 at 9 54 54 AM](https://user-images.githubusercontent.com/75757829/111506458-e1a25d80-8706-11eb-84ae-853b10929832.png)
New flag:
![Screen Shot 2021-03-17 at 9 52 56 AM](https://user-images.githubusercontent.com/75757829/111506474-e49d4e00-8706-11eb-8443-437c0a7dbb88.png)
No flag:
![Screen Shot 2021-03-17 at 9 54 12 AM](https://user-images.githubusercontent.com/75757829/111506469-e36c2100-8706-11eb-843a-0b7a15fe1b2c.png)