tools: update rollup to 2.58.0 #40281
Conversation
iam-frankqiu
added
the
author ready
|
I imagine this is obsoleted by 8a58fce. If not, be sure to run |
Actually, it's not. I have already run |
What makes you say that it isn't? Here's why I think it is. We're already using rollup 2.58.0 on the master branch: $ git checkout master
Already on 'master'
Your branch is up-to-date with 'origin/master'.
$ cd tools/lint-md
$ npm ci
added 184 packages, and audited 185 packages in 2s
133 packages are looking for funding
run `npm fund` for details
found 0 vulnerabilities
$ npm ls rollup
lint-md@1.0.0 /Users/trott/io.js/tools/lint-md
├─┬ @rollup/plugin-commonjs@20.0.0
│ ├─┬ @rollup/pluginutils@3.1.0
│ │ └── rollup@2.58.0 deduped
│ └── rollup@2.58.0 deduped
├─┬ @rollup/plugin-node-resolve@13.0.5
│ └── rollup@2.58.0 deduped
└── rollup@2.58.0
$ |
|
Not sure why it's being weird in the GitHub interface, but the diff against master just shows that the package.json (and the copy of package.json in package-lock.json) are updated but none of the actual package versions in package-lock.json are updated: diff --git a/tools/lint-md/package-lock.json b/tools/lint-md/package-lock.json
index 14ac153c4c..dfc8f3194b 100644
--- a/tools/lint-md/package-lock.json
+++ b/tools/lint-md/package-lock.json
@@ -18,7 +18,7 @@
"devDependencies": {
"@rollup/plugin-commonjs": "^20.0.0",
"@rollup/plugin-node-resolve": "^13.0.5",
- "rollup": "^2.57.0"
+ "rollup": "^2.58.0"
}
},
"node_modules/@rollup/plugin-commonjs": {
diff --git a/tools/lint-md/package.json b/tools/lint-md/package.json
index 60c68f41ee..ffa82b4b14 100644
--- a/tools/lint-md/package.json
+++ b/tools/lint-md/package.json
@@ -16,6 +16,6 @@
"devDependencies": {
"@rollup/plugin-commonjs": "^20.0.0",
"@rollup/plugin-node-resolve": "^13.0.5",
- "rollup": "^2.57.0"
+ "rollup": "^2.58.0"
}
}
|
|
(To see the diff above, make sure |
iam-frankqiu
removed
the
author ready
That's why I said above. It's also pretty confused me. |
iam-frankqiu
force-pushed
the
rollup258
branch
from
0adad61 to
55f939f
Compare
There was a problem hiding this comment.
--- a/tools/lint-md/package.jsonThat's why I said above. It's also pretty confused me.
I can see why it's confusing.
The package.json update has no effect. On current master branch, it lists rollup ^2.57.0 which means 2.x where x is at least 57. The package-lock.json file that's there guarantees that 2.58.0 is installed. The make lint-md-rollup process then uses rollup 2.58.0 to create tools/lint-md/lint-md.js. This is all on current master branch.
All this change does is update the package.json but it has no effect on any code or on the installation of any dependencies including rollup. (See above. We're already using 2.58.0 on master.) This doesn't "update" rollup. It just changes the package.json (and the copy of the package.json that is in package-lock.json.) Those changes have no effect.
Again, I can understand why this is confusing. The creation of the rollup-generated linter is not a matter of simply using npm to update dependencies in tools/lint-md and I don't want to start landing changes that suggest it is.
|
I don't see the point in blocking this PR… We should always keep our versions up-to-date in the |
I'm not sure I agree with that, but I'm not sure I disagree either, and I know I don't want to have to think about it. So sure, let's land it. The commit message should be revised to accurately reflect the change, since this doesn't (anymore) update rollup, so I'll do that while landing it now. |
Update package.json to indicate that rollup 2.58.0 is the minimum. That version was already installed by a previous commit. PR-URL: nodejs#40281 Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
|
Landed in dd3c4a5 |
Update package.json to indicate that rollup 2.58.0 is the minimum. That version was already installed by a previous commit. PR-URL: nodejs#40281 Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
To avoid this, I think we should always update both |
Update package.json to indicate that rollup 2.58.0 is the minimum. That version was already installed by a previous commit. PR-URL: #40281 Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
No description provided.