Skip to content

Conversation

lrlna
Copy link
Contributor

@lrlna lrlna commented Jun 26, 2020

Description

  • adds mongosh_version (from package.json) to getConnectInfo function return.
  • mongosh_version is then sent to analytics and gets logged to logger

@lrlna lrlna changed the title log out mongosh_version to log file MONGOSH-240: log out mongosh_version to log file Jun 26, 2020
Copy link
Collaborator

@mcasimir mcasimir left a comment

Choose a reason for hiding this comment

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

👍

async getConnectionInfo(): Promise<any> {
const buildInfo = await this.buildInfo();
const topology = await this.getTopology();
const { version } = require('../package.json');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we use 1 source of truth for this? Say we would start to publish only packages that are changing this may cause a sneaky bug. Looks good anyway!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did consider that, more specifically keeping the version on shell-internal-state. However, that leads to a very weird API where this particular function getConnectionInfo() would then take version as a param. This does not make sense.

Alternatively, I have also considered storing it in the cli-service-provider and browser-service-provider, which also did not seem logical.

Finally, I have considered doing this require directly in the module that puts all the connection information together, but that would lead to have to change tests in that module every time we bump the version.

This seemed like the most straightforward approach.

As for your point about publishing packages separately -- I think if we end up doing that we will likely want to split this out of the monorepo and redo our entire build process, and if we do that, this will have to be rewritten anyways.

@mcasimir mcasimir merged commit 2916a10 into master Jul 3, 2020
@mcasimir mcasimir deleted the log-mongosh-version branch July 3, 2020 08:31
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.

2 participants