Skip to content

chore: remove configFile and state.json #60

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

Merged
merged 9 commits into from
Apr 11, 2025
Merged

Conversation

fmenezes
Copy link
Collaborator

@fmenezes fmenezes commented Apr 11, 2025

simplifying for now, we should be able to add them later

@fmenezes fmenezes marked this pull request as ready for review April 11, 2025 11:30
src/state.ts Outdated
return false;
}
}
connectionString?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove this - since it's no longer persisted, it is redundant.

@@ -1,34 +1,8 @@
import { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver";
import { AsyncEntry } from "@napi-rs/keyring";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove this package dependency if we're not going to use it.

src/config.ts Outdated
localDataPath,
configPath,
};
fs.mkdirSync(logPath, { recursive: true });
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated to the refactor but we should handle this call failing as we might not have permissions etc. and this is outside our try/catch

@nirinchev
Copy link
Collaborator

Looks like this is now conflicting with the integration tests. You can remove the mocks from helpers.ts since those methods no longer exist.

Copy link
Collaborator

@gagik gagik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be better for the apiClient to have a state rather than for a state to have an apiClient? I could explore this in #55

@fmenezes
Copy link
Collaborator Author

Could it be better for the apiClient to have a state rather than for a state to have an apiClient?

same principle for the connection provider of MongoDB tools, seems like this state is a session rather than state

@fmenezes fmenezes merged commit 0becdaa into main Apr 11, 2025
4 checks passed
@fmenezes fmenezes deleted the fmenezes/removeConfigFile branch April 11, 2025 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants