Repository navigation
Conversation
|
typo in commit message and title |
itsoyou
changed the title
6ccfd81 to
b870236
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #293 +/- ##
==========================================
- Coverage 81.07% 81.07% -0.01%
==========================================
Files 509 509
Lines 73728 73728
Branches 11193 11193
==========================================
- Hits 59775 59773 -2
- Misses 10631 10632 +1
- Partials 3322 3323 +1 see 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
|
@famedly-agent review I know python is not enabled yet but I want to see how it reacts :) |
![famedly-agent[bot]](https://avatars.githubusercontent.com/u/46558835?s=60&v=4){kind=small}
There was a problem hiding this comment.
Nothing that needs to change before merging, just a few smaller notes.
Smaller things
Removed force-cache kills picker on failed fetch · minor · was already like this before this PR · docs/website_files/version-picker.js#L57-L58
Dropping cache: "force-cache" means once the cached gh-pages response goes stale, any failed revalidation — GitHub outage, proxy, or rate limit — rejects the fetch; the top-level .then has no rejection handler, so initializeVersionDropdown never runs and the dropdown stays empty and unclickable. The missing rejection handling predates this PR, so this only affects repeat visitors who previously still saw the stale cached list.
Suggestion: If showing a stale version list is acceptable, restore cache: "force-cache", or add rejection handling on the top-level promise so the dropdown still initializes.
Version fetch treats error responses as success · minor · was already like this before this PR · docs/website_files/version-picker.js#L57-L61
The fetch chain has no res.ok check, so a 403 rate-limit or 404 response still resolves and resObject.tree is undefined; the .filter throws, the catch re-rejects, and the call site has no rejection handler, leaving the version picker empty with no click handler until a reload succeeds. This gap predates the PR, but dropping cache:"force-cache" means every page load now reaches it.
Suggestion: Add an res.ok (or Array.isArray(resObject.tree)) guard before filtering and a .catch at the call site, so a failed fetch leaves the picker in a usable state.
| window.addEventListener("load", () => { | ||
|
|
||
| fetch("https://api.github.com/repos/famedly/synapse/git/trees/gh-pages", { | ||
| cache: "force-cache", |
There was a problem hiding this comment.
looks like the same is going on upstream: https://github.com/element-hq/synapse/blob/develop/docs/website_files/version-picker.js#L58
you might want to have a similar PR there :)
| dropdownMenu.style.display = (dropdownMenu.style.display === 'block') ? 'none' : 'block'; | ||
| }); | ||
|
|
There was a problem hiding this comment.
not sure why you're having all those formatting lines, but maybe better removing them to avoid more issues during the release process?
This branch has not been deployed
3 participants
SYN-147