Escape # in CompareCommits base and head refs#1649
Conversation
google-cla
Bot
added
the
cla: yes
Codecov Report
@@ Coverage Diff @@
## master #1649 +/- ##
=======================================
Coverage 70.01% 70.01%
=======================================
Files 97 97
Lines 6340 6340
=======================================
Hits 4439 4439
Misses 985 985
Partials 916 916
Continue to review full report at Codecov.
|
gmlewis
left a comment
gmlewis
left a comment
There was a problem hiding this comment.
Please add a unit test that demonstrates this new behavior and also add a comment to the method that describes this behavior (between lines 225-226).
Thank you!
|
Hi @gmlewis , I have added the changes you requested. Please let me know if it's okay now. |
gmlewis
left a comment
gmlewis
left a comment
There was a problem hiding this comment.
Why don't I see %23 anywhere in the updated unit test?
Shouldn't the #b# or #h# be escaped?
| for _, c := range cases { | ||
| t.Run(c.name, func(t *testing.T) { | ||
| b := url.QueryEscape(c.b) | ||
| h := url.QueryEscape(c.h) |
There was a problem hiding this comment.
Hi, acutally I store the escaped value for head and base in h and b and use the variables in the test instead of using literal values.
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
gmlewis
left a comment
gmlewis
left a comment
There was a problem hiding this comment.
Thank you, @urohit011 !
LGTM.
Awaiting second LGTM before merging.
|
Thank you, @wesleimp ! |
|
This, unfortunately, causes problems that I should have caught. Reverting. |
3 participants
This change will fix #1642 by replacig
#with%23in the base and head refs which are required for comparing commits.Fixes #1642.