Page MenuHomePhabricator

[e.cash] Retrieve the latest version from github and use it on the upgrade page
ClosedPublic

Authored by Fabien on Apr 5 2024, 13:58.

Details

Summary

Fetch the latest version from the Github API.

Test Plan
npm install
npm run dev

Check the latest version is displayed.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
fetch_latest_version
Lint
Lint Skipped
Unit
No Test Coverage
Build Status
Buildable 28352
Build 56246: Build Diff
Build 56245: arc lint + arc unit

Event Timeline

Fix async/await, no new dependency

Fabien published this revision for review.Apr 5 2024, 14:47
Fabien edited the summary of this revision. (Show Details)
bytesofman requested changes to this revision.Apr 5 2024, 14:56
bytesofman added a subscriber: bytesofman.

With this implementation, we are getitng the version every time this page loads. This means that, as new versions are released, the version here will change (although the other copy on this page will not change).

Is that really the behavior we want?

Seems sloppy to keep it hard coded, but also it ensures that the rest of the code on this page makes sense with the hardcoded version.

web/e.cash/pages/upgrade.js
13 ↗(On Diff #46880)

can we also get the "n -1" version from the API?

17 ↗(On Diff #46880)

should include some error handling here in case the API is down or we do not get the response we expect

let response
try {
response = await fetch...
releases =...
setLatestVersion...
}
catch (err) {
console.error(`Error fetching latest version from github`, err)
}

in this way, if we hit an API error, or if the response is the wrong shape and we don't get the name, we just use the default state field and the page does not crash

This revision now requires changes to proceed.Apr 5 2024, 14:56
web/e.cash/pages/upgrade.js
13 ↗(On Diff #46880)

I plan to do this in a follow-up, it's not n-1 but previous minor so it requires using semantic version comparison. Also it's updated once every 6 month so less of a problem.

17 ↗(On Diff #46880)

Using the getStaticProps it is expected to fail the build if the call fails which is the expected behavior, so this should not be needed

This revision is now accepted and ready to land.Apr 5 2024, 16:58