yaptide / ui

Yet Another Particle Transport IDE - frontend
https://yaptide.github.io/web_dev/
GNU General Public License v3.0
9 stars 4 forks source link

nodejs bump in CI #1533

Closed grzanka closed 6 months ago

grzanka commented 6 months ago

/deploy

github-actions[bot] commented 6 months ago

On-demand deploy: succeeded ✅ https://github.com/yaptide/ui/actions/runs/8282826903

MatoKnap commented 6 months ago

I have a couple of concerns:

grzanka commented 6 months ago

I have a couple of concerns:

  • In .github/workflows/manual_test.yml you removed python-version 3.9 from strategy matrix. In Requirements it is stated that Python 3.9 and higher. We should change Requirements or change back strategy matrix.
  • In .github/workflows/node.js.yml only one Python version is tested even though multiple version of node are tested using strategy matrix. Should we change that to be the same with manual test?

Testing of python wasn't the main point of the tests. Its related to the converter python module which is being used here via webassembly. I would ideally stick to the latest versions (3.12?) here and cover multiple python version in the tests of the converter itself. I don't see much profit of testing compatibility with converter compiled to wheel file on multiple python versions.

I think we could rephrase the README.txt that we need to have a whatever python version which is being able to produce a wheel file for converter. It may be 3.9-3.12. To ensure that wheel is properly produced and working we have enough tests in the converter itself.