Skip to content
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

Change update checker version source #90742

Merged
merged 1 commit into from
May 2, 2024

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Apr 16, 2024

#75916 (comment)
Don't merge until the page is actually up :D

@KoBeWi KoBeWi added this to the 4.3 milestone Apr 16, 2024
@KoBeWi KoBeWi requested a review from a team as a code owner April 16, 2024 08:49
@KoBeWi KoBeWi force-pushed the PR_from_the_future branch from 8c025ed to 68ced81 Compare April 16, 2024 09:22
@coppolaemilio
Copy link
Member

The page https://godotengine.org/versions.yml should serve the file needed. @KoBeWi please test it out to see if there is anything wrong/missing.

@akien-mga
Copy link
Member

akien-mga commented Apr 16, 2024

It's a bit more work on the website side, but shouldn't we expose the data as an easier-to-parse JSON so we can simplify the logic (and third-party project managers / hubs could also use it via our JSON parser, instead of manually implementing a YAML one)?

Likewise, if we include the date info, we might want to make it machine parseable instead of the current hardcoded strings that get reused as is by the blog.

@coppolaemilio
Copy link
Member

I'm fine with either, I think json is more web-friendly, so we should probably use that. Happy to make the adjustments needed, but would like to get at least specifications to generate the file with all the information required.

@KoBeWi
Copy link
Member Author

KoBeWi commented Apr 16, 2024

The file loads fine.

We could replace it with JSON, but technically parsing the file line-by-line is more efficient (with JSON you need to parse the whole file and then search for data). JSON would make the code simpler, but since it's already written, it's fine either way.

As for the specs, it would be best if it was an array of dictionaries, sorted from newest to oldest. Each dictionary needs a version number and flavor/name. The download archive link can be generated from version and release notes are unused, so they don't need to be included.

@akien-mga
Copy link
Member

The download archive link can be generated from version and release notes are unused, so they don't need to be included.

I think the release notes could be used in the future, or by other users of this file, to link directly to release notes. So I'd suggest including them.

As for the download, I'd suggest including it too, in case we ever change the URLs for it.
Currently it's constructed as "https://godotengine.org/download/archive/" + found_version, but if we change this in the future, having it in the JSON means we can just update the JSON and all past Godot versions will still have working links instead of 404s.

Alternatively, we could include just the download archive URL "https://godotengine.org/download/archive/" as a field in the JSON, that can be read by any user and used to compose the full URL for each version, provided we're committed to always have such URLs like "URL_BASE/4.3-dev5".

@GreatNovaDragon
Copy link

I would muse, for further Future Proofing, it would be nice if the actual URL checked is an editor setting, kinda like how you can determine where to download the export templates in that URL?

@coppolaemilio
Copy link
Member

I made the simplest conversion and the file is now a json file: https://godotengine.org/versions.json
If any other field should be included it can be added to the regular _data/versions.yml.

Let me know if further changes are needed!

@akien-mga
Copy link
Member

I made the simplest conversion and the file is now a json file: https://godotengine.org/versions.json If any other field should be included it can be added to the regular _data/versions.yml.

Let me know if further changes are needed!

Looks good! For ease of use, should we add the https://godotengine.org prefix to the release notes fields?

@coppolaemilio
Copy link
Member

They all include the full path in the json file now 👍

@KoBeWi KoBeWi force-pushed the PR_from_the_future branch from 68ced81 to 6e9dc0e Compare May 2, 2024 13:32
@KoBeWi
Copy link
Member Author

KoBeWi commented May 2, 2024

Updated to use the new JSON.

@akien-mga akien-mga merged commit 8f2ff22 into godotengine:master May 2, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the PR_from_the_future branch May 2, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants