google / git-appraise

Distributed code review system for Git repos
Apache License 2.0
5.12k stars 145 forks source link

Add coloring for review state in "git appraise list" #93

Open llorens opened 5 years ago

llorens commented 5 years ago

This commit adds different color highlighting to different review states in the output of "git-appraise list".

The default colors are:

They can be modified by the user by setting the corresponding configuration options in the color.appraise section. E.g. to use magenta for the "pending" state, set color.appraise.pending to "magenta":

 [color "appraise"]
     pending = magenta
googlebot commented 5 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers
llorens commented 5 years ago

Hi, I just recently stumbled upon git-appraise and it immediately seemed a brilliant idea to me. I can imagine I could use it e.g. for one-on-one collaboration/tutoring for junior members of my team, but... The junior members usually have little experience with git and so I would like to work a bit on the usability of git-appraise before I give it a shot. One of the areas where I think git-appraise could be improved is output coloring. I put together some code to add colors to the different states a code review can be in. Could you have a look before I invest more time in supporting other places? This is the first time I've been writing go and the language still feels very awkward to me, so I'll be grateful for any comments! Thanks!

ojarjur commented 5 years ago

@llorens Thanks for taking this on!

I hope git-appraise does work for your use case, and if you hit any issues, please let us know.

We do need a signed CLA before we can accept any of your changes, though.

The comment from "googlebot" above gives the URL you would need to go to in order to sign it.

Thanks.

llorens commented 5 years ago

OK, just signed the CLA.

googlebot commented 5 years ago

CLAs look good, thanks!

llorens commented 5 years ago

I'd wait with merging this. git appraise list -a got noticeably slower as a consequence of this change because git config is called twice for every review listed to get the coloring and color reset sequences. If you have a good idea how to solve this, I'll be happy to hear. Maybe adding output.GetColorSettings() which would return a map and then passing this map to output.PrintSummaryWithColor()? Unless it's OK to add a new parameter to output.PrintSummary()?

ojarjur commented 5 years ago

@llorens Thanks for the warning.

My suggestion matches with what you mentioned; pass a map of colors to a new PrintSummaryWithColor method.

We could then load that map once inside of the commands/list.go and commands/show.go files.