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

feat: introduce -p CLI parameter to output beautified reports #901

Merged
merged 4 commits into from
May 12, 2021

Conversation

lionel-nj
Copy link
Contributor

closes #838

Summary:

This PR provides support for an additional CLI parameter : -p/ --pretty which takes no value. This is used to bautfy the generated reports.

Expected behavior:

Sample usage

-u http://webapps.thebus.org/transitdata/Production/google_transit.zip -t 4 -p -o output

report.json

Capture d’écran, le 2021-05-11 à 15 06 33

system_errors.json

Capture d’écran, le 2021-05-11 à 15 06 41

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with gradle test to make sure you didn't break anything
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

Copy link
Collaborator

@aababilov aababilov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

@@ -36,6 +36,8 @@
public class NoticeContainer {
private static final Gson DEFAULT_GSON =
new GsonBuilder().serializeNulls().serializeSpecialFloatingPointValues().create();
private static final Gson PRETTY_GSON =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We risk to have those Gson objects out of sync. Let's try this:

private static final Gson PRETTY_GSON = new GsonBuilder(DEFAULT_GSON).setPrettyPrinting().create();

Copy link
Contributor Author

@lionel-nj lionel-nj May 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the constructor GsonBuilder(Gson gson) is package-private and cannot be accessed from NoticeContainer class. What do you think of the following?

  private static final Gson PRETTY_GSON = DEFAULT_GSON.newBuilder().setPrettyPrinting().create();

https://www.javadoc.io/doc/com.google.code.gson/gson/2.8.5/com/google/gson/Gson.html#newBuilder--

- remove new additional unit tests
- sync Gson builders
@lionel-nj lionel-nj requested a review from aababilov May 12, 2021 17:44
Copy link
Collaborator

@aababilov aababilov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! But please move the static functions back to their previous location.

@lionel-nj lionel-nj requested a review from aababilov May 12, 2021 23:18
Copy link
Collaborator

@aababilov aababilov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good, thank you!

@aababilov aababilov merged commit 9ac15f6 into master May 12, 2021
@aababilov aababilov deleted the feat/pretty-print branch May 12, 2021 23:28
@isabelle-dr isabelle-dr added this to the v3.0.0 milestone Oct 28, 2021
@isabelle-dr isabelle-dr mentioned this pull request Oct 28, 2021
4 tasks
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.

Add command-line parameter to output "pretty" JSON
3 participants