Support JSON output in report command #102

timofurrer · 2016-02-29T05:57:37Z

Use --json switch to change report output from console output to JSON.

jmaupetit · 2016-02-29T11:06:24Z

Thank you @timofurrer ! JSON output seems a relevant feature. Can you share a concrete use case with us? I mean to which program do you pipe this output to?

Side question: did you update commands.md with make docs?

timofurrer · 2016-03-01T01:56:47Z

I don't have a specific tool at the moment. However, I plan to write an adapter for some company specific time tracking tools ;) .. and beside that I thought others might have the same idea so I just implemented it .

Yeah, I documented it -> See changeset. Is something wrong about it?

jmaupetit · 2016-03-01T09:51:12Z

I plan to write an adapter for some company specific time tracking tools

I am glad that you prefer using Watson than the time tracker of your company ❤️

Yeah, I documented it -> See changeset. Is something wrong about it?

Yep, saw it. That is why I asked. Nothing wrong, just noise.

SpotlightKid · 2016-04-14T16:59:00Z

docs/user-guide/commands.md

@@ -341,6 +366,7 @@ Flag | Help
 `-t, --to DATE` | The date at which the report should stop (inclusive). Defaults to tomorrow.
 `-p, --project TEXT` | Reports activity only for the given project. You can add other projects by using this option several times.
 `-T, --tag TEXT` | Reports activity only for frames containing the given tag. You can add several tags by using this option multiple times
+`-j, --json` | Outputs the report as JSON instead of writing to the console


JSON output still goes to the console, doesn't it? Just the format changes from formatted text with escape sequences to JSON. How can this be reworded to be more accurate?

SpotlightKid · 2016-06-15T20:51:38Z

@timofurrer Do you still plan to update this PR according to the feedback?

timofurrer · 2016-06-15T20:57:52Z

Completely forgot about this PR. I'll have a look at it asap.

yloiseau · 2016-06-29T07:52:53Z

docs/user-guide/commands.md

    apollo11 - 13h 22m 20s
            [brakes    7h 53m 18s]
            [module    7h 41m 41s]
            [reactor   8h 35m 50s]
            [steering 10h 33m 37s]
            [wheels   10h 11m 35s]

+    $ watson report --format json


Here you document --format json while the actual option is --json.
BTW, I think the --format is better since it leaves room to future other formats without adding lots of distinct options.

timofurrer · 2017-04-04T11:38:28Z

It's definitely been a while - I apologize for that!

I've updated the PR and reworded the option description.
I've used --format [rich|json] instead of --json. I'm not quiet happy with the term rich though. Any suggestions?

We could also enhance the current implementation with the if output_format == 'json' by implementing some kind of formatter objects ...
However, I'd wait for that to implement until there is a need for other formats than just json (like CSV or whatever ...)

SpotlightKid · 2017-04-04T11:57:35Z

I'm not quiet happy with the term rich though. Any suggestions?

Umm, not sure, how about formatted or plaintext or plain?

willdurand · 2017-04-04T12:21:55Z

I would keep --json as it is short and useful. There is no need for other formats yet, so we can keep things simple. And it does not mean --format will not be implemented in the future.

timofurrer · 2017-04-04T13:50:35Z

Umm, not sure, how about formatted or plaintext or plain?

Well, the json output is formatted, too. And it's not really plaintext because of the colors.

I would keep --json as it is short and useful. There is no need for other formats yet, so we can keep things simple. And it does not mean --format will not be implemented in the future.

What about a shortcut to --format json? I agree, it does not make a lot of sense to expose an API (--format) which is not really used (well, at this point it is used for some extend, but since we only have to choices - we could consider it a flag)

I can do either one. Who makes the call?

jmaupetit · 2017-04-05T08:49:48Z

Thanks for the update @timofurrer ! I would also keep the --json flag and we will add a shortcut later when the --format api will make sense.

timofurrer · 2017-04-05T17:27:05Z

@jmaupetit @willdurand I've updated the PR according to your suggestion to use --json instead of --format.

jmaupetit

Thank you for this contribution! Can you please use double quotes for translatable strings only? I have mentioned this point in your tests, but I see that there are a few left in your code.

jmaupetit · 2017-04-06T15:55:55Z

tests/test_watson.py

+    assert len(report["projects"]) == 1
+    assert report["projects"][0]["name"] == "foo"
+    assert len(report["projects"][0]["tags"]) == 1
+    assert report["projects"][0]["tags"][0]["name"] == "B"


Can you please use single quotes for non-translatable strings?

jmaupetit · 2017-04-06T16:00:10Z

watson/cli.py

@@ -325,8 +325,11 @@ def status(watson):
              help="Reports activity only for frames containing the given "
              "tag. You can add several tags by using this option multiple "
              "times")
+@click.option('-j', '--json', 'format_json', is_flag=True,
+              help='Format the report in JSON instead of plaintext')


s/plaintext/plain text/

jmaupetit · 2017-04-06T16:00:53Z

watson/cli.py

@@ -346,6 +349,9 @@ def report(watson, current, from_, to, projects, tags, year, month, week, day):
    `--tag` options. They can be specified several times each to add multiple
    projects or tags to the report.

+    You can change the output format for the report from *plaintext* to *JSON*


Idem: s/plaintext/plain text/

timofurrer · 2017-04-06T17:16:25Z

@jmaupetit okay.

I'm not quiet sure what you mean with translatable strings - I couldn't find a consistent pattern for the use of " and ' ...

But I guessed and pushed - Have a look at it 🍻

jmaupetit · 2017-04-10T09:58:03Z

Thank you for these changes. I am sorry to insist, but there are some double quotes left in cli.py (l. 430-467). Once fixed, I'll merge this PR. 💪

timofurrer · 2017-04-10T10:16:32Z

Done.

I think some clear guide lines would really help people to contribute more efficiently ... ;)

willdurand · 2017-04-10T10:28:50Z

I think some clear guide lines would really help people to contribute more efficiently ... ;)

I do agree with you, especially here since I do not know what translatable strings are either. Maybe @jmaupetit could give an explanation here, and we will update the CONTRIBUTING file/doc accordingly.

jmaupetit · 2017-04-10T13:25:53Z

@timofurrer I am sorry if you felt frustrated after submitting code to this PR. This coding convention is implicit, but required to maintain consistency over the project. We need to work on this.

To briefly explain it, translatable strings are literal strings containing natural language messages that may be translated during the internationalization of the app. All other strings acting as "identifiers" are single quoted (dictionary keys, etc.)

timofurrer force-pushed the feature/json-report branch from 80ab323 to 9a2ebe4 Compare February 29, 2016 06:07

timofurrer mentioned this pull request Feb 29, 2016

Set flake8 configuration to allow source code lines longer than 80 chars #105

Closed

jmaupetit added improvement in progress labels Feb 29, 2016

jmaupetit changed the title ~~Support JSON output in report command~~ [WIP] Support JSON output in report command Mar 1, 2016

SpotlightKid reviewed Apr 14, 2016
View reviewed changes

yloiseau reviewed Jun 29, 2016
View reviewed changes

SpotlightKid mentioned this pull request Aug 13, 2016

Current work-week shortcut & progress bar #52

Closed

timofurrer force-pushed the feature/json-report branch from 9a2ebe4 to f1dce0f Compare April 4, 2017 11:35

Support JSON formatted output in report command

6512130

timofurrer force-pushed the feature/json-report branch from f1dce0f to 6512130 Compare April 4, 2017 11:36

timofurrer changed the title ~~[WIP] Support JSON output in report command~~ Support JSON output in report command Apr 4, 2017

timofurrer force-pushed the feature/json-report branch from e3e1fdf to f648349 Compare April 5, 2017 17:25

jmaupetit requested changes Apr 6, 2017

View reviewed changes

timofurrer force-pushed the feature/json-report branch 2 times, most recently from ca5941c to 2505db8 Compare April 6, 2017 17:15

Use -j/--json cli flag instead of --format

66a3782

timofurrer force-pushed the feature/json-report branch from 2505db8 to 66a3782 Compare April 10, 2017 10:15

jmaupetit mentioned this pull request Apr 10, 2017

Add a coding style section to the "contributing" documentation #148

Open

jmaupetit approved these changes Apr 10, 2017

View reviewed changes

jmaupetit merged commit 3c51020 into jazzband:master Apr 10, 2017

jmaupetit removed the in progress label Apr 10, 2017

jmaupetit mentioned this pull request Jul 31, 2017

Bump release to 1.5.0 #155

Merged

davidag mentioned this pull request May 9, 2019

Support CSV output where JSON is allowed #281

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support JSON output in report command #102

Support JSON output in report command #102

timofurrer commented Feb 29, 2016

jmaupetit commented Feb 29, 2016

timofurrer commented Mar 1, 2016

jmaupetit commented Mar 1, 2016

SpotlightKid Apr 14, 2016

SpotlightKid commented Jun 15, 2016

timofurrer commented Jun 15, 2016

yloiseau Jun 29, 2016

timofurrer commented Apr 4, 2017 •

edited

Loading

SpotlightKid commented Apr 4, 2017

willdurand commented Apr 4, 2017

timofurrer commented Apr 4, 2017 •

edited

Loading

jmaupetit commented Apr 5, 2017

timofurrer commented Apr 5, 2017

jmaupetit left a comment

jmaupetit Apr 6, 2017

jmaupetit Apr 6, 2017

jmaupetit Apr 6, 2017

timofurrer commented Apr 6, 2017 •

edited

Loading

jmaupetit commented Apr 10, 2017

timofurrer commented Apr 10, 2017 •

edited

Loading

willdurand commented Apr 10, 2017

jmaupetit commented Apr 10, 2017

Support JSON output in report command #102

Support JSON output in report command #102

Conversation

timofurrer commented Feb 29, 2016

jmaupetit commented Feb 29, 2016

timofurrer commented Mar 1, 2016

jmaupetit commented Mar 1, 2016

SpotlightKid Apr 14, 2016

Choose a reason for hiding this comment

SpotlightKid commented Jun 15, 2016

timofurrer commented Jun 15, 2016

yloiseau Jun 29, 2016

Choose a reason for hiding this comment

timofurrer commented Apr 4, 2017 • edited Loading

SpotlightKid commented Apr 4, 2017

willdurand commented Apr 4, 2017

timofurrer commented Apr 4, 2017 • edited Loading

jmaupetit commented Apr 5, 2017

timofurrer commented Apr 5, 2017

jmaupetit left a comment

Choose a reason for hiding this comment

jmaupetit Apr 6, 2017

Choose a reason for hiding this comment

jmaupetit Apr 6, 2017

Choose a reason for hiding this comment

jmaupetit Apr 6, 2017

Choose a reason for hiding this comment

timofurrer commented Apr 6, 2017 • edited Loading

jmaupetit commented Apr 10, 2017

timofurrer commented Apr 10, 2017 • edited Loading

willdurand commented Apr 10, 2017

jmaupetit commented Apr 10, 2017

timofurrer commented Apr 4, 2017 •

edited

Loading

timofurrer commented Apr 4, 2017 •

edited

Loading

timofurrer commented Apr 6, 2017 •

edited

Loading

timofurrer commented Apr 10, 2017 •

edited

Loading