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

JSON output writer #28

Closed
wants to merge 5 commits into from
Closed

Conversation

WyriHaximus
Copy link
Collaborator

@WyriHaximus WyriHaximus commented Apr 9, 2018

Implements / Closes #24
Closes #29

TODO:

  • Extract echoing output into an EchoWriter
  • JsonWriter
  • --json flag

@WyriHaximus WyriHaximus mentioned this pull request Apr 9, 2018
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 799b4fb on WyriHaximus-labs:json-writer into 33e6587 on Brunty:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 799b4fb on WyriHaximus-labs:json-writer into 33e6587 on Brunty:master.

@coveralls
Copy link

coveralls commented Apr 9, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling af93564 on WyriHaximus-labs:json-writer into 33e6587 on Brunty:master.

@WyriHaximus
Copy link
Collaborator Author

@Brunty this PR is now ready for review. It currently outputs a line of JSON per result/error/stats. Changing it to do it in one big JSON object would have meant a bigger change. Up to you if you prefer it per line or one big JSON object. Example output:

screenshot from 2018-04-11 21 59 40

@Brunty
Copy link
Owner

Brunty commented Apr 14, 2018

Hmm, I'll give it some thought, will look at the pros / cons of each - depends on use case, one big object would make it easier to parse / use in another tool for working with the output.

Also wondering about the possibility of a pretty print flag too, not useful for machines, but useful for people perhaps? Using JSON_PRETTY_PRINT in json_encode() http://php.net/manual/en/function.json-encode.php - thoughts? (it's just something I've come across in other tools before, it's handy to not have to rely on having another tool such as piping it to | python -m json.tool

@WyriHaximus
Copy link
Collaborator Author

Hmmm the JSON_PRETTY_PRINT sounds like a good idea, but that will only work on one big object. You know what, I'll file another PR building on that one doing one big object so you can compare both.

@Brunty
Copy link
Owner

Brunty commented Apr 14, 2018

@WyriHaximus - cool, if there's anything I can help with let me know. I've got some downtime today so I'm going to take a look at this PR. Thanks for the work once again <3

WyriHaximus added a commit to WyriHaximus-labs/cigar that referenced this pull request Apr 14, 2018
@WyriHaximus
Copy link
Collaborator Author

@Brunty #29 is up 😎

@Brunty Brunty closed this in #29 May 9, 2018
Brunty pushed a commit that referenced this pull request May 9, 2018
* Extract echoing output into an EchoWriter

* The passed argument on WriterInterface::writeStats must be a bool not a float

* EchoWriter must implement WriterInterface

* JsonWriter

* Added -j/--json option to the cigar bin script

* Taken #28 and refactored it into outputting a single JSON object instead of multiple
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