-
Notifications
You must be signed in to change notification settings - Fork 503
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
add buildSrc/checkVendoredVersions.js to .github/workflows/test.yml #6906
base: master
Are you sure you want to change the base?
Conversation
7b5407a
to
022535e
Compare
022535e
to
445c073
Compare
buildSrc/updateLibs.js
Outdated
// different from the version in our package.json (the git url), we need to make sure that | ||
// the installed version is in sync with the version given in package.json before we write it to | ||
// vendored-versions.json | ||
await $`npm i` |
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.
I wouldn't add npm i
here, that would catch me off-guard. We should take whatever is already there imo
buildSrc/updateLibs.js
Outdated
.map((d) => (typeof d === "string" ? d : d.src)) | ||
.map(path.normalize) | ||
.map((srcPath) => srcPath.split(path.sep)[2]) | ||
const versions = JSON.parse(fs.readFileSync(path.join(__dirname, "..", "package.json"), "utf-8")).dependencies |
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.
we do read versions from package-json multiple times but I am wondering if we should even take the resolved versions with npm list
or such (would also make it more reliable if we remove npm i
here)
40b934e
to
a3ef88b
Compare
we have a two-stage approach to vendoring libraries: * update the packages' version in package.json * run node buildSrc/updateLibs.js using this check in the test github workflow prevents that commits that contain step 1 but not step 2 can be merged to master. close #6901
a3ef88b
to
ba80b61
Compare
we have a two-stage approach to vendoring libraries:
using this check in the test github workflow prevents that commits that contain step 1 but not step 2 can be merged to master.