-
Notifications
You must be signed in to change notification settings - Fork 270
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
[Blueprints] setSiteLanguage step – download the latest RC translations for Nightly and Beta builds of WordPress #1987
Changes from 9 commits
367e32b
b28ea9d
0b975df
317671b
c9590cf
fc73235
848fe8c
8660c14
df4d543
0d062ea
8cf6394
a339b9a
57658e6
f1f5842
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,26 @@ export async function getLoadedWordPressVersion( | |
return versionStringToLoadedWordPressVersion(versionString); | ||
} | ||
|
||
/** | ||
* Returns a WordPress build version string supported by Playground, | ||
* for a given WordPress version string. | ||
* | ||
* Playground supports the last 4 major.minor versions of WordPress, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is incorrect, though. There's nothing in this function that limits the returned version to the last 4 major releases. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True, I wanted to explain what supported means with that sentence, but it's actually not relevant. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed the mention of support in 0d062ea because it's not relevant. |
||
* the latest beta or RC release (depending on which is newer), | ||
* and nightly releases. | ||
* You can find the full list of supported build versions in | ||
* packages/playground/wordpress-builds/src/wordpress/wp-versions.json | ||
* | ||
* Each released version will be converted to the major.minor format. | ||
* For example 6.6.1 will be converted to 6.6. | ||
* | ||
* Release candidates (RC) and beta releases are converted to "beta". | ||
* | ||
* Nightly releases are converted to "nightly". | ||
* | ||
* @param wpVersionString - A WordPress version string. | ||
* @returns A Playground WordPress build version. | ||
*/ | ||
export function versionStringToLoadedWordPressVersion( | ||
wpVersionString: string | ||
): string { | ||
|
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.
Note that if
wpVersion
is"woah-this-is-cool"
thenwpVersionString
will also be"woah-this-is-cool"
. What would be a sensible fallback here? Perhaps a dedicated version conversion logic would make more sense here?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.
8cf6394 will default translations to the latest WP version to ensure we can always have a valid URL.
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.
Since this already uses involved patterns, calling versionStringToLoadedWordPressVersion seem to add complexity. What would this look like without that call?
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 would look something like this f1f5842
I like this approach more, relying on
versionStringToLoadedWordPressVersion
could lead to issues if the function is changed, especially without tests.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.
Lovely, looking good. Thank you!