Skip to content

chore: switch to @mongodb-js/device-id #196

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 6 commits into from
May 8, 2025
Merged

Conversation

gagik
Copy link
Collaborator

@gagik gagik commented May 5, 2025

This package is meant to minimize code replication and to make sure the approach we're taking with hashing is consistent and timeouts are automatically handled.

See mongodb-js/devtools-shared#532

This package is meant to minimize code replication and to make sure the approach we're taking with hashing is consistent.
@gagik gagik requested a review from a team as a code owner May 5, 2025 11:42
@@ -1,5 +1,5 @@
/** @type {import('ts-jest').JestConfigWithTsJest} **/
export default {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I ran into jestjs/jest#15312, possibly because of a TypeScript update, so just going to keep it as a .cjs file for the time being

Copy link
Collaborator

@blva blva left a comment

Choose a reason for hiding this comment

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

thanks!

this.deviceIdPromise = getDeviceId({
getMachineId: () => this.getRawMachineId(),
onError: (reason, error) => {
switch (reason) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

honestly seeing this used like this does make me think we should instead throw typed error objects and check instanceof as reason feels a bit redundant but this is fine

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I was debating between the two for a while - ended up going with a reason because that was more explicit about the possible values as opposed to Error inheritors, where you'd need to figure out what the options are.

@nirinchev nirinchev enabled auto-merge (squash) May 8, 2025 14:02
@nirinchev nirinchev merged commit 7b033ad into main May 8, 2025
16 checks passed
@nirinchev nirinchev deleted the gagik/use-device-id branch May 8, 2025 14:02
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.

None yet

3 participants