Skip to content
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

make it more difficult to get commits that are out of sync with vendored libraries onto master #6901

Open
ganthern opened this issue Apr 29, 2024 · 0 comments · May be fixed by #6906
Open
Assignees
Labels
build issues that concern the build process (not only ci) maintenance dependency updates, key renewals, code cleanup

Comments

@ganthern
Copy link
Contributor

Currently, it's very easy to update the version on some dependency and then forget to run node buildSrc/updateLibs.js, which results in the old version still running in production.

We should add some mechanism that enforces these files to be in sync with package.json. I can see two main ways to achieve that:

  1. also generate a libs/vendored-versions.json file that needs to be committed when the libs change, then check if it's in sync with package.json during ci
  2. alternatively, run updateLibs and check that the vendored files did not change.

1. vendored versions file

Would be easy to implement, but then there's the slight danger of the version file not being committed.

{
   "mithril": "2.2.2",
   "dompurify: "3.1.1",
   ...
}

2. check that running updateLibs is not changing the files anymore

This would be easiest on the developers updating the libs, but it's made more difficult by the fact that the output of updateLibs is not exactly what we're committing:

  • we apply functional patches like removing mithrils Promise polyfill
  • the formatter is run afterwards (we could maybe add prettier to the rollup build that makes the vendored files)

So at least our patches need to be committed as files and automatically applied in updateLibs before we can add a check like this.

@ganthern ganthern added maintenance dependency updates, key renewals, code cleanup build issues that concern the build process (not only ci) labels Apr 29, 2024
ganthern added a commit that referenced this issue Apr 29, 2024
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
@ganthern ganthern self-assigned this Apr 29, 2024
@kib42 kib42 added this to the Technical tasks milestone May 17, 2024
@murilopereirame murilopereirame self-assigned this May 17, 2024
murilopereirame pushed a commit that referenced this issue May 17, 2024
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build issues that concern the build process (not only ci) maintenance dependency updates, key renewals, code cleanup
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants