-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(db): move db constants to other file to avoid circular ref #1858
Conversation
lib/db.js referenced lib/operations/db_ops.js, which in turn referenced lib/db.js to get some constants. Moved constants to their own file, and used dynamic imports to get DB constructor Fixes NODE-1692
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.
👍
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.
LGTM, just one request then
lib/db_constants.js
Outdated
@@ -0,0 +1,10 @@ | |||
'use strict'; |
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.
similar to the other PR, can we just rename this file constants.js
, in the likely event we will end up with non-db related constants. Trying to contain file bloat here.
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.
Do you think that we shouldindividually bring the constants on to the Db
constructor instead of using Object.assign
? If we add new constants, it will be easy to accidentally expose them.
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.
yeah, I think that is probably best practice
…db#1858) * fix(db): move db constants to other file to avoid circular ref lib/db.js referenced lib/operations/db_ops.js, which in turn referenced lib/db.js to get some constants. Moved constants to their own file, and used dynamic imports to get DB constructor Fixes NODE-1692
lib/db.js referenced lib/operations/db_ops.js, which in turn
referenced lib/db.js to get some constants. Moved constants to
their own file, and used dynamic imports to get DB constructor
Fixes NODE-1692
Replaces #1828