Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

'Requested changes' icon too aggressive #990

Closed
alecbz opened this issue Aug 5, 2017 · 3 comments
Closed

'Requested changes' icon too aggressive #990

alecbz opened this issue Aug 5, 2017 · 3 comments

Comments

@alecbz
Copy link

alecbz commented Aug 5, 2017

Feature request: The white X in a red circle icon that shows up when a PR reviewer requests changes feels too aggressive. It almost looks like the reviewer is saying that the PR shouldn't go through at all, or that something has gone wrong to get into this state. IMO: getting suggestions on how to change code is a normal and healthy part of the code review process, and there shouldn't be an implication that something bad has happened when this occurs.

A softer, more neutral, "warning"-y icon seems like it'd be more appropriate.

@alecbz
Copy link
Author

alecbz commented Sep 9, 2017

I learned something that made me change my tune on this a bit, from https://help.github.com/articles/about-pull-request-reviews/:

If required reviews are enabled and a collaborator with write, admin, or owner access to the repository submits a review requesting changes, the pull request cannot be merged until the same collaborator submits another review approving the changes in the pull request.

This means, AIUI, that even if another reviewer approves the PR, the existence of a "Changes requested" review prevents merging. This is in fact a more aggressive action than I'd originally thought, and makes the red X seem somewhat more appropriate in my mind.

@lieut-data
Copy link

Seems like GitHub has recently rolled out changes here:

image

I'm still a fan of the unambiguous red "X", but perhaps this could be useful to distinguish a CI failure from changes requested in aggregate views such as https://github.com/pulls.

@clarkbw
Copy link
Collaborator

clarkbw commented Feb 11, 2019

Yep, this rolled out last month.
https://github.blog/changelog/2019-01-14-updated-request-changes-icon

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants