search

Genymobile / gm_pr

A multi project Github pull request viewer
Apache License 2.0
41 stars 15 forks source link

Remove feedback weak #77

Closed sgaland closed 8 years ago

sgaland commented 8 years ago

As explained in http://wiki.genymobile.com/index.php5/Github_Workflow

It seems nobody is using it anyway so we should remove this to have a lot more readable output.

CedricCabessa commented 8 years ago

LGTM for the code, but let's discuss this on #dev to first edit the wiki, then update the code

sgaland commented 8 years ago

The wiki already says that we should not use it. And nobody use it. Showing this in gm_pr is not mandatory we should remove it, we can remove it from the wiki as well.

qfarizon commented 8 years ago

LGTM

mmaret-geny commented 8 years ago

:x: I use it, and find it usefull

sgaland commented 8 years ago

@mmaret-geny where? And how many are you to use it? Last time i checked i didn't find one.

mmaret-geny commented 8 years ago

As you know, I'm working in the genymaster team. And, in this team, we have app dev and system dev. Some PR are on the application part, and, as a sys dev, I do not get the full picture. So sometimes, I use it to show to our 2 app dev that they are not alone and that I've look at there work. In fact nothing new from the original design ....

mmaret-geny commented 8 years ago

But, is your concern is really readability, you can display the icon of every sign only if count > 0

sgaland commented 8 years ago

It's ok to show them in the comments of the PR but there is no need for a feedback on gm_pr. But i don't think this is a useful information and it should not appear on the PR summary screenshot from 2016-02-18 09-13-34 Even if some one put a :hand: it's gonna take all the visual space even if it's not important.

But as always, i won't force change, if you all like it that way, fine by me.

CedricCabessa commented 8 years ago

show if count > 0 looks good, project that never use it will never see it And you did a similar work for slack a long time ago https://github.com/Genymobile/gm_pr/pull/46/files

sgaland commented 8 years ago

working on it

sgaland commented 8 years ago

https://github.com/Genymobile/gm_pr/pull/78