-
Notifications
You must be signed in to change notification settings - Fork 65
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
feat: v21 #413
Conversation
👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labled with |
It seems that the test is failing because of a trailing slash (
|
@octokit/js Is anyone able to figure out how to get the test passing? Is there a way to get |
I'm going to take a look into this. I'll keep you posted. |
TL;DR (in
|
Request | Response |
---|---|
/repos/OWNER/REPO/branches |
200 |
/repos/OWNER/REPO/branches/ |
404 |
But, /repos/OWNER/REPO/contents/PATH
works with and without /
, so we should probably support both 1.
Request | Response |
---|---|
/repos/octokit/endpoint.js/contents |
200 |
/repos/octokit/endpoint.js/contents/ |
200 |
/repos/octokit/endpoint.js/contents/test |
200 |
/repos/octokit/endpoint.js/contents/test/ |
200 |
This mismatch comes when we compare the requested URL (with no ending /
) and the expected from @octokit/fixtures
(with ending /
):
https://github.com/octokit/fixtures/blob/5aa66e46e3f3475c4c1eb521cd402dfd85091b52/scenarios/api.github.com/get-content/normalized-fixture.json#L5
We should probably improve the logic we have in place in @octokit/endpoints
, but I would like to hear your thoughts on this @octokit/js @octokit/extensibility-sdks. I'm discarding the option of fixing it in @octokit/fixtures
because those are auto-generated.
Footnotes
-
Apparently in the past the trailing
/
was not working for/repos/{org}/{repo}/contents
endpoint but seems that it was escalated and now it's working. ↩
The Problem (in
|
BREAKING CHANGE: package is now ESM