ci: migrate to shared workflows#300
Conversation
| for arg in $@; do | ||
| case ${arg} in | ||
| -i|--install) | ||
| npm install;; |
There was a problem hiding this comment.
Shouldn't it run npm ci as before?
There was a problem hiding this comment.
I am wondering if we should add both: ci to be used in the GH action, but install locally in case you want to bump dependencies.
There was a problem hiding this comment.
I feel that -i flag here is just for the CI purposes, for local dev, you could always do npm install from root here, to update package-lock
* Replaces js-tests with shared inveniosoftware/workflows * Updates Node test matrix to 22.x, 24.x, 26.x * Adds run-js-linter.sh and run-js-tests.sh scripts * Removes conflicting react-app lint rules * Updates Node requirement to >=22.0.0
mirekys
force-pushed
the
mirekys/ci-migration
branch
from
b902c75 to
0f313ec
Compare
| uses: inveniosoftware/workflows/.github/workflows/tests-js.yml@master | ||
| with: | ||
| js-working-directory: ./ | ||
| node-version: '["22.x", "24.x", "26.x"]' |
There was a problem hiding this comment.
nit: should we test with 3 nodejs versions? for python we use an upper and lower bound?
There was a problem hiding this comment.
Hmm I think it makes sense to test every LTS version starting from our lower bound 22.x 🤔 Do you know the reason why it was chosen to only test upper and lower bound in the python modules?
| "search", | ||
| "react search" | ||
| ], | ||
| "eslintConfig": { |
There was a problem hiding this comment.
Any reason why we are deleting this ESLint config?
There was a problem hiding this comment.
Because .eslintrc.yml already imports react-app https://github.com/inveniosoftware/eslint-config-invenio/blob/master/configs/main.yaml
There was a problem hiding this comment.
Yes it was kinda redundant with .eslintrc.yml already present here: https://github.com/oarepo/react-searchkit/blob/014bc1124937f840acfa43120677e0fabaf82eda/.eslintrc.yml
5 participants
Description
Part of: inveniosoftware/rfcs#112
Checklist
Ticks in all boxes and 🟢 on all GitHub actions status checks are required to merge:
Frontend
Reminder
By using GitHub, you have already agreed to the GitHub’s Terms of Service including that: