-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
chore: dependencies version consistency #2235
chore: dependencies version consistency #2235
Conversation
.github/workflows/pr-lint.yaml
Outdated
- name: Install syncpack | ||
run: pnpm add --save-dev syncpack |
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.
Better to have this as a development dependency on the monorepo, so we can version it properly. And we have the config file committed anyways.
.github/workflows/pr-lint.yaml
Outdated
- name: Install Dependencies | ||
run: pnpm install | ||
|
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.
- name: Install Dependencies | |
run: pnpm install |
Would suggest removing this as ./.github/actions/ci-setup
already performs an install 😄
d8d9146
to
087125f
Compare
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.
Thanks for your work on this.
A few things:
- Seems you need to run
pnpm install
as the lock file is not in sync with the package.json - I'd suggest adding a command in the
package.json
to run the job akin to thedepcheck
command that you can call from from the CI job. - I'm going to put the PR in draft, you can re-open it once all the CI tests pass, but whilst they are failing please use the logs to inform you on how best to proceed.
f745640
to
64128ff
Compare
Hi @maschad, is this good enough? |
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.
Hi @richardgreg I think this is a great start. To finish up:
- you may want to define a syncpack config to avoid mismatches such as
workspace:*
- Apparently there is a Github action which might save you some trouble
After that you can fix the mismatches then this should be ready for review
package.json
Outdated
@@ -110,8 +111,9 @@ | |||
"vite": "^5.2.10", | |||
"vite-plugin-node-polyfills": "^0.21.0", | |||
"vite-plugin-plain-text": "^1.4.2", | |||
"vitest": "^1.6.0", | |||
"webdriverio": "^8.36.1" | |||
"vitest": "^1.1.3", |
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.
Did you intend to downgrade "vitest"
? I suppose this is an accident
39d4f5e
to
6b90879
Compare
Also abstracted the logic for checking mismatches to a script.
Also gave ci system permission to execute check-deps.sh
6b90879
to
e97529f
Compare
9b44bae
to
e8fc80a
Compare
Hi @maschad, I've made the modifications. |
Thanks for this work @richardgreg I must apologize for an oversight on my part as it seems you would need elevated permissions to be able to run this workflow accurately, which is why you are getting this error I will take over the PR from here, merging your changes into one of my branches. Thanks for all of your work! |
Hi @maschad, do you have any other issues of this calibre in mind that I can work on? |
Hi @richardgreg feel free to look through our good first issues and see if there's anything that piques your interest. |
Superseded by #2883 Thank you @richardgreg 🚀 |
Fixes #2028