chore: type improvement for schedulers #6136
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 2 Ignored Deployments
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Code Health Quality Gates: OK
- Declining Code Health: 1 findings(s) 🚩
- Improving Code Health: 0 findings(s) ✅
- Affected Hotspots: 0 files(s) 🔥
Recommended Review Level: Lightweight sanity check
View detailed results in CodeScene
🚩 Declining Code Health (highest to lowest):
- Large Method schedule-services.ts: scheduleServices
| @@ -48,6 +48,7 @@ export const scheduleServices = async ( | |||
| apiTokenService.fetchActiveTokens.bind(apiTokenService), | |||
| minutesToMilliseconds(1), | |||
| 'fetchActiveTokens', | |||
| 0, // no jitter -> run immediately | |||
There was a problem hiding this comment.
❌ Getting worse: Large Method
scheduleServices increases from 135 to 136 lines of code, threshold = 70
Why does this problem occur?
Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function. Read more.
To ignore this warning click here.
| () => { | ||
| clientMetricsServiceV2.bulkAdd().catch(console.error); | ||
| async () => { | ||
| await clientMetricsServiceV2.bulkAdd().catch(console.error); |
There was a problem hiding this comment.
() => clientMetricsServiceV2.bulkAdd().catch(console.error); is almost the same since the last statement and returns Promise. The main difference is that it returns Promise with results value and not just Promise
There was a problem hiding this comment.
but there's no return... I could add one, right?
There was a problem hiding this comment.
In general it's more idiomatic to do () => Promise.resolve() over async () => { await Promise.resolve(); }
There was a problem hiding this comment.
() => Promise.resolve() returns the promise when you don't add {} around it
There was a problem hiding this comment.
() => Promise.resolve() is more idiomatic than () => { return Promise.resolve() }
| () => { | ||
| clientMetricsServiceV2.clearMetrics(48).catch(console.error); | ||
| async () => { | ||
| await clientMetricsServiceV2.clearMetrics(48).catch(console.error); |
I saw we were making an await on a function that didn't return a promise, so I thought maybe there's something off that's causing our scheduled jobs to pile up. So I wanted to change the type to find out that, but I don't think there's something... |
About the changes
Closes #
Important files
Discussion points