-
Notifications
You must be signed in to change notification settings - Fork 3.4k
chore: add knip health checks to repo #32994
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
Conversation
cypress
|
||||||||||||||||||||||||||||||||||||||||
| Project |
cypress
|
| Branch Review |
knip-integration
|
| Run status |
|
| Run duration | 18m 44s |
| Commit |
|
| Committer | Jennifer Shehane |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
8
|
|
|
1097
|
|
|
4
|
|
|
26698
|
| View all changes introduced in this branch ↗︎ | |
Warning
Partial Report: The results for the Application Quality reports may be incomplete.
UI Coverage
45.76%
|
|
|---|---|
|
|
187
|
|
|
162
|
Accessibility
98.01%
|
|
|---|---|
|
|
4 critical
8 serious
2 moderate
2 minor
|
|
|
101
|
This comment was marked as duplicate.
This comment was marked as duplicate.
| }, | ||
| "devDependencies": { | ||
| "@babel/types": "7.28.2", | ||
| "@jest/globals": "^30.1.2", |
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.
Bug: Unrelated @jest/globals dependency added to data-context
The PR is described as adding knip health checks, yet it includes an unrelated addition of @jest/globals to the data-context package devDependencies. This appears to be an accidental or unintended change that got included with the knip-related modifications.
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.
This is unrelated! This is used in this package however and it was correlated to so many errors that I added it to reduce the errors.
| "gulpfile.js" | ||
| ] | ||
| }, | ||
| "cli": { |
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.
These entrypoints do seem like they'll be a bit unreasonable to keep up with. Probably a sign of our own monorepo not being so well organized. Maybe esm migration will help with this. You can kind of tell which packages are most unruly just by the amount of rules or ignores needed.
| "size": "t=\"cypress-v0.0.0.tgz\"; yarn pack --filename \"${t}\"; wc -c \"${t}\"; tar tvf \"${t}\"; rm \"${t}\";", | ||
| "test": "yarn test-unit", | ||
| "test-debug": "npx vitest --inspect-brk --no-file-parallelism --test-timeout=0", | ||
| "test-dependencies": "dependency-check . --missing --no-dev --verbose", |
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'm pretty sure this wasn't even being called anymore, but this was the intention - to remove this dep.
AtofStryker
left a comment
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.
looks good and we can tune it without having to worry about disruptions
dependency-check#30117Additional details
Add health-check command to root of repo that runs knip checks. View current health-check here: https://app.circleci.com/pipelines/github/cypress-io/cypress/77438/workflows/98f4f680-9225-4f53-b1c7-8bb36140e40e/jobs/3322448
Right now, since much of the feedback hasn't been addressed and we may need to tweak the config still, knip always:
Currently this is a job that one could inspect in CI or run locally to further improve the health of the repo in subsequent PRs.
Steps to test
yarnyarn health-checkHow has the user experience changed?
N/A
PR Tasks
cypress-documentation?type definitions?Note
Adds a Knip-based health check with config and CI job, makes it required in workflows, removes legacy dependency-check script, and updates dev deps.
health-checkjob runningyarn health-checkto.circlecipipelines (@pipeline.yml,workflows/@main.yml,workflows/pull-request.yml) and make it a prerequisite forready-to-release.knip.jsonand root scripthealth-check(knip --no-exit-code); addknipto devDependencies.test-dependenciesscript anddependency-checkdevDependency; keeppostinstallscript.@jest/globalsas a devDependency inpackages/data-context.yarn.lockwith Knip and related transitive dependencies.Written by Cursor Bugbot for commit 8280c92. This will update automatically on new commits. Configure here.