-
Notifications
You must be signed in to change notification settings - Fork 501
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
feat: test npm build&install against LTS node versions #1962
Conversation
Currently, the workflow is set up where the badge will show RED (failure) if unable to be built successfully by Linux, Windows, AND macOS. We could have separate badges for each platform (you should see some of this in the commit history) but I think it's not worth the effort, because we want to support all three platforms, correct? So if one of the platforms can't build successfully using a specific version of node, then the repo doesn't actually support that version. Let me know if there's a different way we should handle this. |
.github/workflows/node-test.yml
Outdated
reset-badge: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: "Reset ${{ inputs.gh-node-version }} badge" | ||
uses: RubbaBoy/BYOB@v1.2.1 | ||
with: | ||
ICON: https://raw.githubusercontent.com/devicons/devicon/master/icons/nodejs/nodejs-original.svg | ||
NAME: "node-${{ inputs.gh-node-version }}" | ||
LABEL: "${{ inputs.gh-node-version }}" | ||
STATUS: "Building..." | ||
COLOR: grey | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
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.
this may not be necessary, I'm open to removing it.
package.json
Outdated
@@ -138,6 +139,7 @@ | |||
"jest-playwright-preset": "^1.7.0", | |||
"jest-process-manager": "^0.3.1", | |||
"multihashing-async": "^1.0.0", | |||
"node-pre-gyp": "^0.17.0", |
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.
node-pre-gyp is deprecated -- that's why @mapbox/node-pre-gyp
was added -- but this repo wouldn't build on github-actions without it. not sure if you have a known fix for this.
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.
The proper fix is to update all dependencies so node-pre-gyp
is no longer pulled-in. Unfortunately, for that we need to update js-ipfs dependencies to the latest version: #1840
Using @mapbox/node-pre-gyp
here sounds like a good band-aid until that happens 👍
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 think one badge per nodejs version is more than enough.
It is already a bit overkill, but since it runs once a week it should be ok.
Small suggestions in comments inline, but feel free to merge either way.
package.json
Outdated
@@ -138,6 +139,7 @@ | |||
"jest-playwright-preset": "^1.7.0", | |||
"jest-process-manager": "^0.3.1", | |||
"multihashing-async": "^1.0.0", | |||
"node-pre-gyp": "^0.17.0", |
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.
The proper fix is to update all dependencies so node-pre-gyp
is no longer pulled-in. Unfortunately, for that we need to update js-ipfs dependencies to the latest version: #1840
Using @mapbox/node-pre-gyp
here sounds like a good band-aid until that happens 👍
this new workflow also publishes shields, which would then be published on our README.
README update:
workflow results (via https://github.com/SgtPooki/ipfs-webui/actions/runs/2605014389)