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

DXCDT-315: Refactor log streams create and update cmds #599

Merged
merged 3 commits into from
Jan 12, 2023

Conversation

sergiught
Copy link
Contributor

@sergiught sergiught commented Jan 11, 2023

🔧 Changes

Refactoring auth0 logs streams create|update to have an extra subcommand to create a specific log stream type that only has a subset of the flags actually needed for that log stream e.g. auth0 logs streams create datadog [flags].

Screen.Recording.2023-01-11.at.19.08.20.mov

📚 References

🔬 Testing

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@sergiught sergiught self-assigned this Jan 11, 2023
@sergiught sergiught force-pushed the patch/DXCDT-315-logstreams-refactor branch 2 times, most recently from 48df9ad to fd9a90f Compare January 11, 2023 17:57
@sergiught sergiught force-pushed the patch/DXCDT-315-logstreams-refactor branch from fd9a90f to e96be9a Compare January 11, 2023 18:07
@sergiught sergiught marked this pull request as ready for review January 11, 2023 18:12
@sergiught sergiught requested a review from a team as a code owner January 11, 2023 18:12
@Widcket
Copy link
Contributor

Widcket commented Jan 11, 2023

Shouldn't the --json flag result in colored output, unless --no-color is specified?

Screenshot 2023-01-11 at 15 18 26

@Widcket
Copy link
Contributor

Widcket commented Jan 11, 2023

I notice in some commands this % symbol, seems like a renderer method is missing a \n.

Screenshot 2023-01-11 at 15 20 09

@sergiught
Copy link
Contributor Author

Shouldn't the --json flag result in colored output, unless --no-color is specified?

@Widcket: Good catch! That should be indeed the case. I wonder if #594 had unintended consequences. We'll fix this in a separate PR as it's unrelated to the log streams command restructuring. Thanks for spotting this!

I notice in some commands this % symbol, seems like a renderer method is missing a \n.

IMO when --json flag is present we shouldn't end with a new line as that will mess up the input for tools like jq.

This is how it will look like with an additional new line after the json output (to remove the % symbol):

image

This is how it looks like without the new line:

image

@Widcket
Copy link
Contributor

Widcket commented Jan 11, 2023

IMO when --json flag is present we shouldn't end with a new line as that will mess up the input for tools like jq.

Yes, that's right.

internal/cli/log_streams.go Outdated Show resolved Hide resolved
internal/cli/log_streams.go Outdated Show resolved Hide resolved
internal/cli/log_streams_datadog.go Outdated Show resolved Hide resolved
internal/cli/log_streams_event_bridge.go Outdated Show resolved Hide resolved
internal/cli/log_streams_http.go Show resolved Hide resolved
@sergiught sergiught merged commit 37e0321 into v1 Jan 12, 2023
@sergiught sergiught deleted the patch/DXCDT-315-logstreams-refactor branch January 12, 2023 18:45
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.

3 participants