-
-
Notifications
You must be signed in to change notification settings - Fork 740
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
Instance stats service composition root #6029
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
@@ -20,7 +33,7 @@ afterAll(async () => { | |||
}); | |||
|
|||
test('should return instance statistics', async () => { | |||
stores.featureToggleStore.create('default', { | |||
await stores.featureToggleStore.create('default', { |
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.
missing await
{ | ||
experimental: { | ||
flags: { | ||
strictSchemaValidation: true, |
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.
missing schema validation
@@ -97,6 +98,7 @@ export default async function init( | |||
getLogger, | |||
}); | |||
|
|||
log.setLogLevel('error'); |
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.
stop logging db migration messages in all tests
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.
Awesome! This was something I was planning to do yesterday 🥇
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.
Nice one! Yeah that's a lot of dependencies 😅
@@ -97,6 +98,7 @@ export default async function init( | |||
getLogger, | |||
}); | |||
|
|||
log.setLogLevel('error'); |
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.
Awesome! This was something I was planning to do yesterday 🥇
const instanceStatsService = db | ||
? createInstanceStatsService(db, config) | ||
: createFakeInstanceStatsService(config); |
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.
Neat!
About the changes
Ability to create a dependency graph of the instance stats service in one method call. This PR shows high coupling to 10+ stores we have in this part of the code.
Important files
Discussion points