Refactor/2025 09 delete unused property #544
Conversation
takapi327
added
🔧 refactor
There was a problem hiding this comment.
Pull Request Overview
Refactors the PooledDataSource to remove unused background fiber properties and simplify resource management. The PR eliminates abstract method definitions for background fibers that were not being used and restructures the resource creation logic.
Key changes:
- Remove unused background fiber properties from trait and implementation
- Simplify resource creation with dedicated helper methods
- Refactor for-comprehension for cleaner resource management
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| backgroundTasks.sequence_.as(pool) | ||
| } | ||
| )(_ => pool.close) // Ensure background tasks are stopped before closing the pool |
There was a problem hiding this comment.
The comment mentions 'background tasks' but this resource only handles minimum connections creation. The comment should be updated to reflect that this finalizer closes the pool after minimum connections are no longer needed.
| )(_ => pool.close) // Ensure background tasks are stopped before closing the pool | |
| )(_ => pool.close) // Close the pool after minimum connections are no longer needed |
| keepaliveExecutor.map(_.start(pool)), | ||
| statusReporter.map(_.start(pool, config.poolName)) | ||
| ).flatten | ||
| backgroundResources.sequence_ |
There was a problem hiding this comment.
The createBackgroundResources method returns F[Unit] from sequence_ but should return Resource[F, Unit]. This should be wrapped in a resource to properly manage the lifecycle of background fibers, such as Resource.make(backgroundResources.sequence_)(_ => /* cleanup logic */).void.
| backgroundResources.sequence_ | |
| Resource.make( | |
| backgroundResources.sequence_ | |
| )(_ => pool.close) |
There was a problem hiding this comment.
Since backgroundResources itself is of type List[Resource[F, Unit]], calling sequence_ transforms it into Resource[F, Unit], which is normal processing.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## series/0.4.x #544 +/- ##
================================================
- Coverage 83.94% 83.46% -0.49%
================================================
Files 187 187
Lines 8421 8774 +353
Branches 1138 1183 +45
================================================
+ Hits 7069 7323 +254
- Misses 1352 1451 +99
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Implementation Details
Fixes
Fixes #xxxxx
Pull Request Checklist
References