-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Fix comment API paths #2813
Fix comment API paths #2813
Conversation
Can we create new v3 API route group for all these breaking changes? |
@lafriks IMO, that seems like overkill. I doubt that fixing a few broken routes will cause much disruption. |
7850c4a
to
38f4785
Compare
If we are sure no tool is using it I'm ok with that |
38f4785
to
a61cc00
Compare
@lafriks I'm not aware of any such tool, but of course I can't guarantee that no such tool exists. |
a61cc00
to
4802efb
Compare
Codecov Report
@@ Coverage Diff @@
## master #2813 +/- ##
=======================================
Coverage 27.24% 27.24%
=======================================
Files 89 89
Lines 17640 17640
=======================================
Hits 4806 4806
Misses 12146 12146
Partials 688 688 Continue to review full report at Codecov.
|
Well, any way to know if it'll break things is to do it and see if anyone complains LGTM |
4802efb
to
96d1009
Compare
@lafriks Instead of removing the old, incorrect routes, I've marked them as deprecated. This PR is no longer breaking. |
LGTM |
Corrects the following comment API routes
PATCH /:owner/:repo/issues/:index/comments/:id
->PATCH /:owner/:repo/issues/comments/:id
DELETE /:owner/:repo/issues/:index/comments/:id
->DELETE /:owner/:repo/issues/comments/:id
The previous routes ignored the
:index
parameter, and were in conflict with the Github API. The previous route are marked as deprecated, but have not been removed.