Correct the runIDFromURLRE regex to properly match the callbackURL
#3495
Conversation
There was a problem hiding this comment.
I should have caught this before (sorry about that), but this repo supports both public GitHub and enterprise GitHub installations, which means that we can't hard-code a domain name in a regex.
Instead of your change, we should simply remove the leading carat (^) and then it should support both styles of URLs.
Please update this PR and then add a test for both styles of URLs showing that it should work on both. Thank you, @pputman-clabs.
gmlewis
changed the title
gmlewis
changed the title
runIDFromURLRE regex to properly match the callbackURL
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3495 +/- ##
=======================================
Coverage 91.03% 91.03%
=======================================
Files 179 179
Lines 15574 15574
=======================================
Hits 14178 14178
Misses 1223 1223
Partials 173 173 ☔ View full report in Codecov by Sentry. |
…move the carat(^) to match any url, along with tests for both https://api.github.com along with a dummy url for enterprise Github
There was a problem hiding this comment.
One more tweak, please, @pputman-clabs, then we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.
There was a problem hiding this comment.
Thank you, @pputman-clabs!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
|
@stevehipwell - might you have time for a code review? Thank you! |
|
Thank you, @stevehipwell! |
the callbackURL from the event previously didn't match with the regex, and was always causing a "no match found error". This includes the full URL and should match.