-
-
Couldn't load subscription status.
- Fork 487
update pyodide from 0.23.4 to 0.28.1 #1754
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
base: main
Are you sure you want to change the base?
Conversation
There were some significant performance improvements in 0.24: https://blog.pyodide.org/posts/0.24-release/ They aren't particularly helpful for pyodide-in-node, but could be helpful for grist-static. Another development is support for snapshotting which could be a huge help, however this isn't mature enough yet to use.
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 checking these change using the Yunohost package and see whether it works as expected.
My remarks are mostly nit picks. Looks good to me, though this is not the part of the code I know the most.
| @@ -0,0 +1,33 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| # Usage: ./clean_if_new_version.sh N | |||
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.
Hmm, part of me wonders what happens when this script is not called using ./ but from another destination (like the root of the grist-core project).
But maybe it is rather expected that these scripts are called using Make, so not very relevant.
| source env.sh | ||
| cd _build/pyodide | ||
| git checkout 0.23.4 || (git fetch && git checkout 0.23.4) | ||
| git checkout $PYODIDE_VERSION || (git fetch && git checkout $PYODIDE_VERSION) |
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.
Above the script starts with set -e.
What do you think of adding the -u option? I think it can be especially useful when you are using variables:
-u Treat unset variables as an error when substituting.
(usually I work with set -eEuo pipefail which covers many cases of error)
| cd _build/worker | ||
| yarn init --yes | ||
| yarn add pyodide@0.23.4 | ||
| yarn add pyodide@$PYODIDE_VERSION |
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.
Idem, maybe use set -eu above to be sure this variable is set.
| "friendly_traceback-0.7.48-cp313-none-any.whl", | ||
| "iso8601-0.1.12-cp313-none-any.whl", | ||
| "lazy_object_proxy-1.6.0-cp313-cp313-pyodide_2025_0_wasm32.whl", | ||
| "openpyxl-3.0.10-cp313-none-any.whl", |
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.
Are these changes related to the python version? (3.11 -> 3.13)
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.
LGTM (few remarks that may be ignored), and it works with Yunohost.
As I mentionned earlier, not the part of the code I know the most. A second review might be appropriate if you feel like it would need.
There were some significant performance improvements in 0.24:
https://blog.pyodide.org/posts/0.24-release/
They aren't particularly helpful for pyodide-in-node, but could be helpful for grist-static. Another development is support for snapshotting which could be a huge help, however this isn't mature enough yet to use.