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

Add support for multi-line comments. #1566

Merged
merged 3 commits into from Jun 24, 2020
Merged

Conversation

mattmoor
Copy link
Contributor

This adds support for the "comfort-fade" preview, which enables multi-line comments (incl. suggested edits).

I have been carrying a subset of this as a patch downstream for a while, which just hard wired things on, but I tried to make this "smart" for back-compat as part of upstreaming.

This adds support for the "comfort-fade" preview, which enables multi-line comments (incl. suggested edits).

I have been carrying a subset of this as a patch downstream for a while, which just hard wired things on, but I tried to make this "smart" for back-compat as part of upstreaming.
@googlebot googlebot added the cla: yes Indication that the PR author has signed a Google Contributor License Agreement. label Jun 21, 2020
@codecov
Copy link

codecov bot commented Jun 21, 2020

Codecov Report

Merging #1566 into master will decrease coverage by 0.00%.
The diff coverage is 67.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1566      +/-   ##
==========================================
- Coverage   67.90%   67.90%   -0.01%     
==========================================
  Files          95       95              
  Lines        8722     8750      +28     
==========================================
+ Hits         5923     5942      +19     
- Misses       1892     1898       +6     
- Partials      907      910       +3     
Impacted Files Coverage Δ
github/pulls_reviews.go 71.81% <67.85%> (-0.92%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be9fac6...bb38860. Read the comment docs.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @mattmoor!

Do you want to add any comments to the CreateReview method documentation describing how to use this multi-line comments feature? The announcement on https://developer.github.com/changes/2019-10-03-multi-line-comments/ is rather light on documentation.

Your call. Otherwise, one minor tweak please, and then this PR LGTM and we will be ready for a second LGTM and then a merge.

github/pulls_reviews.go Outdated Show resolved Hide resolved
@mattmoor
Copy link
Contributor Author

Yeah, ack on the docs. I spent a few days beating my head against their API (and a few twitter DM convos with their PMs) to figure out the various ways I was holding things wrong.

I added a detailed comment, which reflects my observations (Good old, Dr. Empirical). Hopefully it will keep others from falling into the same traps I did.

Copy link
Collaborator

@gmlewis gmlewis left a comment

Choose a reason for hiding this comment

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

Thank you, @mattmoor!
LGTM.

Awaiting second LGTM before merging.

@mattmoor
Copy link
Contributor Author

@gmlewis Anything I can do to help move this along? 😇

@gmlewis
Copy link
Collaborator

gmlewis commented Jun 24, 2020

You can always ping @wesleimp - he is one of the most responsive volunteer reviewers we have. 😄

Copy link
Collaborator

@wesleimp wesleimp left a comment

Choose a reason for hiding this comment

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

LGTM 👌

@gmlewis
Copy link
Collaborator

gmlewis commented Jun 24, 2020

See what I mean?!? 🤣
Thank you, @wesleimp !
Merging.

@gmlewis gmlewis merged commit 3d244d3 into google:master Jun 24, 2020
n1lesh pushed a commit to n1lesh/go-github that referenced this pull request Oct 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indication that the PR author has signed a Google Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants