-
Notifications
You must be signed in to change notification settings - Fork 128
chore(ci): add dependency checks and reorganize tangled dependencies #576
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
This adds a dep check using `knip`. We use `depcheck` in Compass but this is deprecated and knip is recommend instead. This adds a basic configuration and then uses it to untangle some of our dependencies. This includes: - Moving dependencies into dev dependencies, most notably `mongodb`! which we actually only use for types in the code itself at the moment. - Moving certain types, consts into helper utils as they are only relevant there. This seems like a useful feature and allows us to avoid mixing up our scripting or testing code with production.
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.
Pull Request Overview
This PR introduces dependency management improvements using knip
for dependency checking and reorganizes dependencies to better separate production code from development/testing code.
- Adds
knip
configuration and dependency checking to the CI pipeline - Moves production-only types and utilities like
mongodb
from dependencies to devDependencies - Relocates test-specific types and utilities to a dedicated test utils module
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
knip.json |
Adds knip configuration for dependency analysis |
package.json |
Reorganizes dependencies, moves mongodb and other testing dependencies to devDependencies |
tests/utils/index.ts |
Creates new test utilities module with NullLogger and TestConnectionManager |
tests/utils/elicitationMocks.ts |
Removes unused MockElicitInput type |
tests/integration/helpers.ts |
Moves driverOptions configuration from config module to test helpers |
src/common/logger.ts |
Removes NullLogger class (moved to test utils) |
src/common/connectionManager.ts |
Removes TestConnectionManager type (moved to test utils) |
src/common/config.ts |
Removes driverOptions export (moved to test helpers) |
Various test files | Updates imports to use new test utilities and helper locations |
Comments suppressed due to low confidence (2)
tests/accuracy/sdk/matcher.ts:1
- The removal of
PARAMETER_SCORER_SYMBOL
appears to be removing unused code, but this should be verified with test coverage to ensure this symbol is truly unused across the accuracy testing framework.
const MATCHER_SYMBOL = Symbol("match");
tests/accuracy/sdk/constants.ts:1
- The removal of
TEST_DATA_DUMPS_DIR
constant should be verified to ensure it's not used elsewhere in the accuracy testing framework, as this could break test data loading functionality.
import path from "path";
Ah, it seems trouble figuring out bash scripts... So some deps need to be re-added |
Pull Request Test Coverage Report for Build 17939682178Details
💛 - Coveralls |
@@ -0,0 +1,11 @@ | |||
{ | |||
"entry": [ | |||
"src/index.ts!", |
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.
FYI: for knip !
means it's a production code it treats it differently then entry points files (i.e. that's how it complains about something being a dependency instead of a dev dependency)
"dependencies": { | ||
"@modelcontextprotocol/sdk": "^1.17.4", | ||
"@mongodb-js/device-id": "^0.3.1", | ||
"@mongodb-js/devtools-connect": "^3.9.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.
correct me if I'm wrong but I don't think we were using devtools-connect
anywhere...
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 think it's a transitive dependency of the mongosh service-provider-node-driver. We can get rid of it here.
…gagik/add-depcheck
ec9979f
to
03f3bc5
Compare
0b81cfd
to
b5b80c5
Compare
This adds a dep check using
knip
. We usedepcheck
in Compass but this is deprecated and knip is recommend instead. This adds a basic configuration and then uses it to untangle some of our dependencies. This includes:mongodb
! which we actually only use for types in the code itself at the moment.This seems like a useful feature and allows us to avoid mixing up our scripting or testing code with production.