eschutho commented on a change in pull request #14652:
URL: https://github.com/apache/superset/pull/14652#discussion_r644396157
##########
File path: Makefile
##########
@@ -38,6 +38,22 @@ superset:
# Load some data to play with
superset load-examples
+update:
Review comment:
@nytai `npm install` will also bump dependencies if a new version is
available that matches the given range. Since we don't pin our dependencies, we
could wind up with updates to package-lock.json without any changes to
package.json. Depending on if that's what we want, but my opinion would be for
package-lock.json updates in PRs only to be there when there's an explicit
change in the package.json file. It makes reviews easier. My vote would be to
use `npm ci` for updates, even though it takes longer.
##########
File path: Makefile
##########
@@ -38,6 +38,22 @@ superset:
# Load some data to play with
superset load-examples
+update:
Review comment:
Ha ha.. so I've still been seeing updated package-lock.json files in PRs
that shouldn't be there, but.. I tested it out with v 7.6.0 and pulled Superset
1.0.0 and ran `npm install` and it updated the lockfile. :(
Either way, we can leave this as is, but if we do, I think we should be
extra vigilant about sneaky tailgaiting package-lock files in PRs. Technically
it _should_ do no harm to have them updated, if the package is using semver
correctly, but it just creates more ambiguity and room for potential bugs if
it's there for no seemingly good reason.
##########
File path: Makefile
##########
@@ -38,6 +38,22 @@ superset:
# Load some data to play with
superset load-examples
+update:
Review comment:
I'll still continue to recommend that people use `npm ci` when updating.
--
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
