-
Notifications
You must be signed in to change notification settings - Fork 26
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
full eslint #625
full eslint #625
Conversation
8d60ed1
to
bb46389
Compare
025042c
to
3a48d6c
Compare
3cf92a6
to
9f52c4d
Compare
3a48d6c
to
0649ab4
Compare
cc7a988
to
f4ba129
Compare
d5ae78c
to
c42de2d
Compare
f4ba129
to
a4d06b1
Compare
c42de2d
to
416a523
Compare
a4d06b1
to
47d26f9
Compare
6d9d521
to
7f7eb3b
Compare
58d6e8f
to
c40c62d
Compare
c40c62d
to
a6af60c
Compare
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.
Very nice!
It seems node-webrtc
doesn't support Apple Silicon, indeed on M2 running npm ci
raises this error:
npm ERR! code 1
npm ERR! path /Users/vignoud/Disco/review_repo/node_modules/wrtc
npm ERR! command failed
npm ERR! command sh -c -- node scripts/download-prebuilt.js
npm ERR! response status 404 Not Found on https://node-webrtc.s3.amazonaws.com/wrtc/v0.4.7/Release/darwin-arm64.tar.gz
npm ERR! node-pre-gyp info it worked if it ends with ok
npm ERR! node-pre-gyp info using [email protected]
npm ERR! node-pre-gyp info using [email protected] | darwin | arm64
npm ERR! node-pre-gyp info check checked for "/Users/vignoud/Disco/review_repo/node_modules/wrtc/build/Release/wrtc.node" (not found)
npm ERR! node-pre-gyp http GET https://node-webrtc.s3.amazonaws.com/wrtc/v0.4.7/Release/darwin-arm64.tar.gz
npm ERR! node-pre-gyp ERR! install response status 404 Not Found on https://node-webrtc.s3.amazonaws.com/wrtc/v0.4.7/Release/darwin-arm64.tar.gz
npm ERR! node-pre-gyp ERR! install error
npm ERR! node-pre-gyp ERR! stack Error: response status 404 Not Found on https://node-webrtc.s3.amazonaws.com/wrtc/v0.4.7/Release/darwin-arm64.tar.gz
npm ERR! node-pre-gyp ERR! stack at /Users/vignoud/Disco/review_repo/node_modules/@mapbox/node-pre-gyp/lib/install.js:67:15
npm ERR! node-pre-gyp ERR! stack at processTicksAndRejections (node:internal/process/task_queues:96:5)
npm ERR! node-pre-gyp ERR! System Darwin 23.3.0
npm ERR! node-pre-gyp ERR! command "/Users/vignoud/.nvm/versions/node/v16.20.2/bin/node" "/Users/vignoud/Disco/review_repo/node_modules/.bin/node-pre-gyp" "install"
npm ERR! node-pre-gyp ERR! cwd /Users/vignoud/Disco/review_repo/node_modules/wrtc
npm ERR! node-pre-gyp ERR! node -v v16.20.2
npm ERR! node-pre-gyp ERR! node-pre-gyp -v v1.0.11
npm ERR! node-pre-gyp ERR! not ok
I found a few open issues with a similar installation error:
None of the fixes I tried worked on M2. I didn't try this one.
FYI issue #698 mentions using @koush/wrtc
instead, which I see you removed from the dependencies. I tried npm i @koush/wrtc
anyway but still doesn't work so I don't know what changed compared to when it used to work on my M2.
ho it remaines me that we add that, I though it was an issue of early userspace M2 support and that was fixed in the meantime. looks like it wasn't.
|
7f88162
to
89e39f6
Compare
89e39f6
to
140d23b
Compare
@JulienVig as I don't have a M2 around, can you try out the latest commit plz? :) |
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.
Very nice, the code is much cleaner!
I saw you created a new module isomorphic-wrtc
, could you maybe add a few lines of explanation in the doc about what's the purpose of this module? For example as a README.md in the isomorphic-wrtc
or in the Structure section of DEV.md
discojs/discojs-core/src/dataset/data/preprocessing/tabular_preprocessing.ts
Show resolved
Hide resolved
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.
npm -ws run build
fails because the isomorphic-wrtc doesn't have a build script:
Lifecycle script
build
failed with error:
npm ERR! Error: Missing script: "build"
77f59bd
to
5ca655c
Compare
indeed, it's a bit cryptic when you first come to the project. added some infos in isomorphic-wrtc/README.md
ho right, I didn't check the return code when doing that. added empty scripts for build/lint/test |
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.
Looks good to me!
found out that eslint was not working on much files (ie, only on javascript files that we don't have anymore).