-
Notifications
You must be signed in to change notification settings - Fork 76
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
[Bug] image in readme would crash when default branch is not master #1268
Comments
Thanks for the bug report. |
After looking through the code, looks that's not easy to figure this out. |
I guess we need to use the If you have time to look into that, it would be helpful. |
what if just before this (or in parallel) call to github api val request = new Request(
s"https://api.github.com/repos/$organization/$repository/readme",
new RequestInit {
headers = headersWithCreds.toJSDictionary
}
) One makes this other one |
Yes that should solve the issue! Feel free to open a PR. |
I'll give it a shot! to be fair: it's not the solution... I see it as a workaround but it should a) serve the purpose b) have little impact on the structure of that part of the codebase. |
I'll try and summarize a possible solution. As of now
val request = new Request(
s"https://api.github.com/repos/$organization/$repository/readme", ...)
Now what I had in mind is to keep point 1 and 2 as they and separate the rest. So the first part would look like: // Simplified version
fetch(readmeRequest).toFuture
.flatMap { res =>
res.text().toFuture
}
.foreach(res => element.innerHTML = res) The second one would make the "new" GitHub API request ( So something along these lines: // Simplified version
@js.native
trait Repo extends js.Object {
val default_branch: String = js.native
}
(...)
val repoRequest = new Request(
s"https://api.github.com/repos/$organization/$repository/", ...)
fetch(repoRequest).toFuture
.flatMap(res => res.text().toFuture)
.foreach { res =>
val resJson = js.JSON.parse(res)
val branch = resJson.asInstanceOf[Repo].default_branch
setImagesAndAnchors(branch, organization, repository)
} (And obviously How does this look like? |
Looks good but the second part (or more precisely the |
Cool. I'm just wondering if I've got an easy way to test that this works. Any hint? |
This is how you can run Scaladex locally: https://github.com/scalacenter/scaladex/blob/main/CONTRIBUTING.md#run-scaladex-locally. Then you should go to the |
Working on it have a look mfirry@d960fa3 still trying to understand if can i somehow handle unsuccessful API calls |
@mfirry thanks for the update. It is looking good.
The API call should not fail and if it fails I would use |
Current behavior
image in readme would crash when default branch is not master
see project logo in here
Expected Behavior
image in readme should display
Extra comments
Should tune
Client.scala
rather hard codemaster
Search terms
master
orblob
inClient.scala
The text was updated successfully, but these errors were encountered: