-
Notifications
You must be signed in to change notification settings - Fork 26
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
api: use upstream offered JSON files #1
Conversation
db7a5f2
to
798a290
Compare
@@ -665,14 +665,17 @@ function setModel(overview, target, id) { | |||
function changeModel(version, overview, title, base_url) { | |||
const entry = overview.profiles[title]; | |||
if (entry) { | |||
fetch(`${base_url}/${entry.target}/${entry.id}.json`, { | |||
fetch(`${base_url}/targets/${entry.target}/profiles.json`, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that instead of the individual JSON files, the OpenWrt generated profiles.json files are used. Those file contain all models of the target. This OK.
But we would still need to create the overview.json file. No?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's now outomatocally created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure what automatic means. Is the overview.json now generated by the OpenWrt build process?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that code now part of https://github.com/openwrt/openwrt ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it's part of the way we deploy downloads.openwrt.org
www/index.js
Outdated
if (config.show_snapshots) { | ||
let branches = []; | ||
for (const version of versions.slice()) { | ||
let branch = version.split('.').slice(0, -1).join('.') + '-SNAPSHOT'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not like to hard code this OpenWrt specific stuff. I wonder if this can be done in a nicer way.
Shouldn't we but this in the .versions.json file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure would would you add it?
let base_url = config.versions[version]; | ||
fetch(base_url + "/overview.json", { cache: "no-cache" }) | ||
let base_url = `${config.image_url}/releases/${version}`; | ||
if (version == "SNAPSHOTS") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quiet a hack. Can we avoid to make this project too OpenWrt specific?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, what do you suggest?
@aparcar these changes make things simpler. But I haven't found the concentration to make this change elegant yet. |
Please take your time |
e05942c
to
ffc8328
Compare
This is right now for testing only, however ideally we'd switch to automatically generated JSON files instead of relying on the collect script. Signed-off-by: Paul Spooren <[email protected]>
This is right now for testing only, however ideally we'd switch to automatically generated JSON files instead of relying on the collect script.