search

google / git-appraise

Distributed code review system for Git repos
Apache License 2.0
5.13k stars 146 forks source link

git-appraise list -json dump full review with comments. #82

Open tcolgate opened 6 years ago

tcolgate commented 6 years ago

I'm working on vim integration for git-appraise. g Trying to get a list of reviews programmatically, git-appraise list -json dumps the entirety of all reviews, rather than just the "headline" details. Ideally it will leave dumping of the individual review details to git-appraise show. On busy repositories, git appraise list -a -o json could be prohibitively large I guess?

ojarjur commented 6 years ago

I agree with the general approach you are describing; only do the minimal amount of work necessary to display the information in your UX, and leave other details until the user opens a specific review.

The tool does follow that design (git appraise show --json will display more information than git appraise list --json), but it is probably still including more information than what you actually need for your UX.

We could address this by adding more output options to the list command. For instance, we could have a format flag that allows you to specify the actual fields to output. At the very least, we should probably have an option that shows the same information that is shown in the non-JSON format, but structured as JSON.

BTW, since you mentioned performance on a large repo; I do have a test repository with over 15 thousand reviews in it. The run times I see for listing those reviews are:

$ time git appraise list -a --json | grep '    "revision": ' | wc
  15050   30100  903000

real    0m3.662s
user    0m3.196s
sys     0m1.308s

and

$ time git appraise list -a | grep '^\[' | wc
  15050   30100  316066

real    0m2.656s
user    0m2.196s
sys     0m1.176s

So, the process of outputting the extra JSON and then discarding it does add noticeable latency.