Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Check Run Annotations #1864

Merged
merged 234 commits into from
Nov 27, 2018
Merged

Check Run Annotations #1864

merged 234 commits into from
Nov 27, 2018

Conversation

StanleyGoldman
Copy link
Contributor

@StanleyGoldman StanleyGoldman commented Aug 16, 2018 •
edited
Loading

This pull request adds functionality to:

  • Show a clickable indicator on pull request checks that have annotations
  • Show a list of annotations on a pull request
  • Show a count of annotation failures, warnings and notices in a pull request file list.

Remaining

  • Design work

Before merging

Images

image

image

@StanleyGoldman
Copy link
Contributor Author

@donokuda... send 💕

@StanleyGoldman
Attempting to display an icon for PullRequestCheckViewModels that hav…
d9b6fe5 
…e annotations
@StanleyGoldman
Merge branch 'features/check-suites' into features/check-suite-annota…
fafc68f 
…tions
@StanleyGoldman
Merge branch 'features/check-suites' into features/check-suite-annota…
10aee41 
…tions
@StanleyGoldman
Displaying an icon next to check runs with annotations
220a3e9
@StanleyGoldman
Tweaking min width
3409360
@StanleyGoldman
Merge branch 'features/check-suites' into features/check-suite-annota…
47bc9f1 
…tions

# Conflicts:
#	src/GitHub.InlineReviews/Services/PullRequestSessionService.cs
@StanleyGoldman
Adding a display for PullRequest Annotations
68a1d12
@StanleyGoldman
Functionality to display annotations in a dedicated view
317c4ae
@StanleyGoldman
Fixing error
86feafd
@StanleyGoldman
Fixing formatting
cc4219a
@StanleyGoldman
Adding different types of inline tags
8fb18b4
@StanleyGoldman
Merge branch 'features/check-suites' into features/check-suite-annota…
95d0079 
…tions

# Conflicts:
#	src/GitHub.App/ViewModels/GitHubPane/PullRequestCheckViewModel.cs
#	src/GitHub.VisualStudio/Views/GitHubPane/PullRequestCheckView.xaml
@StanleyGoldman StanleyGoldman force-pushed the features/check-suite-annotations branch from c7ba7d6 to 95d0079 Compare August 20, 2018 19:38
@StanleyGoldman
Merge branch 'features/check-suites' into features/check-suite-annota…
42be1cd 
…tions

# Conflicts:
#	src/GitHub.App/ViewModels/GitHubPane/PullRequestCheckViewModel.cs
#	src/GitHub.InlineReviews/Services/PullRequestSessionService.cs
@StanleyGoldman
Merge branch 'features/check-suites' into features/check-suite-annota…
966c955 
…tions
@meaghanlewis meaghanlewis added this to the 2.5.6 milestone Aug 21, 2018
@StanleyGoldman StanleyGoldman changed the title [WIP] Check Suite Annotations [WIP] Check Run Annotations Aug 22, 2018
@StanleyGoldman StanleyGoldman changed the base branch from features/check-suites to master August 22, 2018 15:59
donokuda and others added 17 commits November 21, 2018 11:17
@donokuda
Merge branch 'features/check-suite-annotations-inline' of https://git…
adfada0 
…hub.com/github/VisualStudio into features/check-suite-annotations-inline
@donokuda
It's a circle now!
e9080ab
@donokuda
Update color
747084c
@donokuda
It's a lil diamond now
a0e2fcd
@StanleyGoldman
Adding additional Check Run fields
0d81d7c
@donokuda
Make everything a diamond
c67c5b4
@donokuda
Fix bottom margin for reply textbox
c110ad9
@StanleyGoldman
Using only one view
271c2cd
@StanleyGoldman
Merge pull request #2077 from github/donokuda/polish-annotation-margin
4069376 
Update inline review / annotation margin glyph
@StanleyGoldman
Merge remote-tracking branch 'remotes/origin/master' into features/ch…
c2d7d72 
…eck-suite-annotations
@StanleyGoldman
Merge branch 'features/check-suite-annotations' into features/check-s…
fe6c09d 
…uite-annotations-inline
@StanleyGoldman
Attempting to control the comment ability
f13d354
@StanleyGoldman
Correcting the xaml binding
f3c4548
@StanleyGoldman
Merge pull request #2080 from github/features/check-suite-annotations…
ce065dc 
…-inline-available-for-comment

Check Run Annotations should be able to disable the Add Comment functionality
@StanleyGoldman
Merge branch 'master' into features/check-suite-annotations
9e9978f
@StanleyGoldman
Merge branch 'features/check-suite-annotations' into features/check-s…
1da4ac2 
…uite-annotations-inline
@grokys
Merge pull request #1900 from github/features/check-suite-annotations…
fef4a16 
…-inline

Check Run Annotations Inline
grokys
grokys reviewed Nov 26, 2018
View reviewed changes
Copy link
Contributor

@grokys grokys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

src/GitHub.Exports/Models/CheckRunModel.cs
@@ -21,7 +28,14 @@ public class CheckRunModel
/// </summary>
public DateTimeOffset? CompletedAt { get; set; }

/// <summary>The name of the check for this check run.</summary>
/// <summary>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test

@grokys
Copy link
Contributor

grokys commented Nov 26, 2018

Couple of things I've noticed with the commenting behavior after this PR:

image

  • Is it intended that both comments and annotations now get a grey diamond icon in the margin? I'd have expected the icon to distinguish between comments and annotations
  • There's now a vertical scrollbar in the comment box that scrolls one pixel

StanleyGoldman and others added 4 commits November 26, 2018 13:47
@StanleyGoldman
Removing margin
e249664
@donokuda
no margins
a6f67e8
@donokuda
Merge branch 'features/check-suite-annotations' of https://github.com…
cd33156 
…/github/VisualStudio into features/check-suite-annotations
@donokuda
remove border from comments
6bbb1ae 
This might help with visually grouping comment threads in the future
@donokuda
Copy link
Contributor

Is it intended that both comments and annotations now get a grey diamond icon in the margin? I'd have expected the icon to distinguish between comments and annotations

This was intended. I was exploring different ways of indicating if a line had an annotation, a comment, both, and what kind of annotation. Here's a crop of a work-in-progress mockup:

image

The challenge I ran into was how to represent more than one annotation marker in the margin and decided on one to show to simplify things. However, I'll work on iterating in @StanleyGoldman's pull request for using different markers.

@donokuda
Copy link
Contributor

Heads up, I pushed a change that removed some border lines between comments. My (future) intent is to group comment threads together and removing borders will make this easier visibly.

image

@grokys grokys merged commit 9124705 into master Nov 27, 2018
@grokys
Copy link
Contributor

grokys commented Nov 27, 2018 •
edited
Loading

Ok, thanks for the details @donokuda - I've merged this, lets iterate on it in separate PRs. Those mockups look great!

@StanleyGoldman StanleyGoldman deleted the features/check-suite-annotations branch November 27, 2018 12:31
@StanleyGoldman StanleyGoldman mentioned this pull request Dec 3, 2018
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@grokys grokys grokys approved these changes

@jcansdale jcansdale Awaiting requested review from jcansdale

Assignees

@StanleyGoldman StanleyGoldman

@donokuda donokuda

Labels
None yet
Projects
None yet
Milestone
2.6.0
Development

Successfully merging this pull request may close these issues.
4 participants
@StanleyGoldman @grokys @donokuda @meaghanlewis