📌 Check in package-lock.json as well #1198
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Ah, it looks like copying the |
So we can have the CI use known good versions of the dependencies. Without this change, the underlying libraries can change based on the loosely defined semver in the `package.json`, which can cause unexpected failures. An example of such a failure is: e-mission/e-mission-docs#1113 Copying over the `package-lock.json` overrides `package.json` and forces the build to use the exact versions specified e-mission/e-mission-docs#1113 (comment) Note that `package-lock.json` is apparently designed to checked in to source control. It used to be checked in (thanks to a contribution from the UNSW team) but I removed it because every run of `npm install` would override it, resulting in confusing merge conflicts. However, it looks like `npm` has now changed its behavior so it respects the `package-lock.json` if it exists. e-mission/e-mission-docs#1113 (comment) So we restore it by copying in known good versions - from https://github.com/e-mission/e-mission-phone/actions/runs/12448538357 for the "serve" case and from the last successful internal build of NREL OpenPATH for the `cordovabuild` case. We also change `bin/configure_xml_and_json.js` to copy these files over as well Testing done: - Copying `package-lock.json` into the repo causes `npm install` to work, and the tests to pass - Testing of the setup changes will happen through CI
shankari
force-pushed
the
force_package_dependencies_fix_tests
branch
from
a278bb0 to
be94357
Compare
|
So this is a bit weird. In the first round of this, I checked in the But using the version that was uploaded from https://github.com/e-mission/e-mission-phone/actions/runs/12448538357 |
Although we tried to override the `package-lock.json`, it is overridden, in turn by npm when we run npm install after changing `package.json` And because we have a `package.serve.json` and a `configure_xml_and_js` script, we change the `package.json` every time we clone, so the `package-lock.json` never stick. #1198 (comment) The fix is to re-copy just the lock file after the first `npm install` and then run `npm install` again, essentially duplicating the flow in #1198 (comment)
Ah I think I have more clarity - the So the flow needs to be:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1198 +/- ##
=======================================
Coverage 29.78% 29.78%
=======================================
Files 123 123
Lines 4955 4955
Branches 1142 1095 -47
=======================================
Hits 1476 1476
- Misses 3477 3479 +2
+ Partials 2 0 -2
Flags with carried forward coverage won't be shown. Click here to find out more. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So we can have the CI use known good versions of the dependencies. Without this change, the underlying libraries can change based on the loosely defined semver in the
package.json, which can cause unexpected failures.An example of such a failure is:
e-mission/e-mission-docs#1113
Copying over the
package-lock.jsonoverridespackage.jsonand forces the build to use the exact versions specified e-mission/e-mission-docs#1113 (comment)Note that
package-lock.jsonis apparently designed to checked in to source control. It used to be checked in (thanks to a contribution from the UNSW team) but I removed it because every run ofnpm installwould override it, resulting in confusing merge conflicts.However, it looks like
npmhas now changed its behavior so it respects thepackage-lock.jsonif it exists.e-mission/e-mission-docs#1113 (comment)
So we restore it by copying in known good versions - from https://github.com/e-mission/e-mission-phone/actions/runs/12448538357 for the "serve" case and from the last successful internal build of NREL OpenPATH for the
cordovabuildcase.We also change
bin/configure_xml_and_json.jsto copy these files over as wellTesting done:
package-lock.jsoninto the repo causesnpm installto work, and the tests to pass