Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pull Request status circle #2145

Merged
merged 12 commits into from Feb 1, 2019
Merged

Pull Request status circle #2145

merged 12 commits into from Feb 1, 2019

Conversation

@StanleyGoldman
Copy link
Contributor

@StanleyGoldman StanleyGoldman commented Dec 19, 2018

My attempt to emulate this in Xaml

image

TODO:

  • Get love from @donokuda
  • Remove the summary checks from the View Model, if we decide not to use it

IMAGE:
image

…p display logic
@jcansdale
Copy link
Collaborator

@jcansdale jcansdale commented Dec 20, 2018

I'd love to see some indication of whether the pull request is in a merge-able state. At the moment we know that ✔️ is definitely and is definitely not merge-able (assuming any restrictions are in place).

Where we see a doughnut (💭 mmm doughnuts), we have no way of knowing whether the pull request is in a merge-able state.

Crazy idea, could we color the center of the doughnut green or red, when we know the pull request is definitely merge-able or not merge-able? 😄

@donokuda
Copy link
Member

@donokuda donokuda commented Jan 11, 2019

I pushed a small tweak adjusting the thickness of the donut chart so that it's closer to the line thickness of the octicons:

screen shot 2019-01-10 at 4 34 57 pm

This looks good to be merged in.

I'd love to see some indication of whether the pull request is in a merge-able state. At the moment we know that ✔️ is definitely and is definitely not merge-able (assuming any restrictions are in place).

Crazy idea, could we color the center of the doughnut green or red, when we know the pull request is definitely merge-able or not merge-able?

@jcansdale I'm hesitant on adding more information since we'll need to come up with an approach that's clear and can fit in the limited amount of space that we have. I struggled with this a bit with the annotation markers; and in the spirit of moving this pull request forward, I think it's best better to punt at this time.

Would you be open to showing this type of information in the pull request detail view / sidebar? That way, we can breakdown why a branch isn't mergeable (out of date, review needed, required checks, etc) and guide developers to fix it:

image

@StanleyGoldman StanleyGoldman changed the title [WIP] Adding a Pull Request status circle User Control Adding a Pull Request status circle User Control Jan 13, 2019
@StanleyGoldman StanleyGoldman removed their assignment Jan 13, 2019
@StanleyGoldman StanleyGoldman requested review from grokys and jcansdale Jan 13, 2019
@meaghanlewis meaghanlewis added this to the 2.7.1 milestone Jan 14, 2019
@meaghanlewis meaghanlewis mentioned this pull request Jan 15, 2019
7 of 15 tasks complete
Copy link
Contributor

@grokys grokys left a comment

Looks great in general. Just a problem with how you're doing dependency properties.

StanleyGoldman and others added 4 commits Jan 25, 2019
…nge functions
Meaghan Lewis
@grokys
grokys approved these changes Feb 1, 2019
@StanleyGoldman StanleyGoldman merged commit 7a69a15 into master Feb 1, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
github.VisualStudio Build #20190131.2 succeeded
Details
@StanleyGoldman StanleyGoldman deleted the pull-request-status-circle branch Feb 1, 2019
@jcansdale jcansdale changed the title Adding a Pull Request status circle User Control Show a Pull Request status circle Feb 11, 2019
@StanleyGoldman StanleyGoldman changed the title Show a Pull Request status circle Pull Request status circle Feb 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.