You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/04/07 05:56:41 UTC

[GitHub] [arrow] domoritz opened a new pull request #9918: ARROW-12239: [JS] Switch to yarn

domoritz opened a new pull request #9918:
URL: https://github.com/apache/arrow/pull/9918


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] domoritz commented on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
domoritz commented on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-815090379


   > Do tools like GH security alerts work with yarn.lock files now too?
   
   Yes, I have seen it with my projects and I only use yarn.
   
   > `npm audit`
   
   There is `yarn audit`. I will switch to that in this pull request. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] kou commented on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
kou commented on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-816317653


   It seems that the `-isystem` error isn't occurred on master.
   Can I rebase this branch on master?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] github-actions[bot] commented on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-814640992


   https://issues.apache.org/jira/browse/ARROW-12239


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] domoritz commented on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
domoritz commented on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-816292196


   @kou There is an error `CMake Error: Unknown argument -isystem` but I don't see how it's related. Any ideas?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] domoritz edited a comment on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
domoritz edited a comment on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-816326899


   Go for it. You can also merge since we probably should squash this pull request anyway. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] trxcllnt commented on a change in pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
trxcllnt commented on a change in pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#discussion_r609024237



##########
File path: js/.npmrc
##########
@@ -1,2 +0,0 @@
-save-prefix=
-engine-strict=true

Review comment:
       Sure but this flag controls behavior if someone does `npm i -D new-dep` (or `yarn add --dev new-dep`).
   
   We don't want npm/yarn writing the default `new-dep@^x.y.z` to the package version in `"devDependencies"`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] trxcllnt commented on a change in pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
trxcllnt commented on a change in pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#discussion_r608810124



##########
File path: js/.npmrc
##########
@@ -1,2 +0,0 @@
-save-prefix=
-engine-strict=true

Review comment:
       IIRC yarn respects `.npmrc` files too, and if so should we keep this?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] domoritz commented on a change in pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
domoritz commented on a change in pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#discussion_r608896400



##########
File path: js/.npmrc
##########
@@ -1,2 +0,0 @@
-save-prefix=
-engine-strict=true

Review comment:
       Dev dependencies are already guarded by the yarn lockfile. But I guess it makes sense to have specific versions so that an npm user doesn't run into problems?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] domoritz commented on a change in pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
domoritz commented on a change in pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#discussion_r608850435



##########
File path: js/.npmrc
##########
@@ -1,2 +0,0 @@
-save-prefix=
-engine-strict=true

Review comment:
       But we can fix that later. I'll add it back for now. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] kou commented on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
kou commented on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-816255857


   It seems that the following error is related to this:
   
   https://github.com/apache/arrow/pull/9918/checks?check_run_id=2298651927#step:7:367
   
   ```text
   error Your lockfile needs to be updated, but yarn was run with `--frozen-lockfile`.
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] trxcllnt commented on a change in pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
trxcllnt commented on a change in pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#discussion_r608890174



##########
File path: js/.npmrc
##########
@@ -1,2 +0,0 @@
-save-prefix=
-engine-strict=true

Review comment:
       Mostly as a guard to catch when we're installing (dev) dependencies. We've promoted most of our prod dependencies to `^` explicitly.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] domoritz commented on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
domoritz commented on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-816326899


   Go for it. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] trxcllnt commented on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
trxcllnt commented on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-815053239


   While I personally love yarn and prefer it over npm, I think we've stuck to using `npm` for two reasons:
   
   1. It ships with node
   2. `npm audit` and related tooling integration 
   
   Do tools like GH security alerts work with `yarn.lock` files now too? If so, 2 may be less of a concern.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] domoritz commented on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
domoritz commented on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-816260639


   @kou thanks! That came from a merge. I updated the lockfile. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] domoritz commented on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
domoritz commented on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-815384036


   The failures look unrelated to my changes. Do you agree @trxcllnt?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] kou closed pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
kou closed pull request #9918:
URL: https://github.com/apache/arrow/pull/9918


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] domoritz edited a comment on pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
domoritz edited a comment on pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#issuecomment-815090379


   > Do tools like GH security alerts work with yarn.lock files now too?
   
   Yes, I have seen it with my projects and I only use yarn.
   
   > `npm audit`
   
   There is `yarn audit`. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [arrow] domoritz commented on a change in pull request #9918: ARROW-12239: [JS] Switch to yarn

Posted by GitBox <gi...@apache.org>.
domoritz commented on a change in pull request #9918:
URL: https://github.com/apache/arrow/pull/9918#discussion_r608850082



##########
File path: js/.npmrc
##########
@@ -1,2 +0,0 @@
-save-prefix=
-engine-strict=true

Review comment:
       Why is it good not to have a save prefix? It means any consumer may not be able to find a version that overlaps with what they already have. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org