Skip to content

Rename MysqlConfig property, correct fromUri default, and remove redundant code#6

Merged
rcalicdan merged 2 commits into
hiblaphp:mainfrom
jeffreybulanadi:dev
May 14, 2026
Merged

Rename MysqlConfig property, correct fromUri default, and remove redundant code#6
rcalicdan merged 2 commits into
hiblaphp:mainfrom
jeffreybulanadi:dev

Conversation

@jeffreybulanadi

@jeffreybulanadi jeffreybulanadi commented May 14, 2026

Copy link
Copy Markdown
Member

Closes #5

Six of the seven approved changes from the issue. Point 8 (adding final to Connection) is not included - Mockery::mock(Connection::class) in tests/Pest.php requires subclassing to build the test double and final blocks that. It needs an interface extraction before it can land.

  • MysqlConfig::fromUri() was defaulting enableServerSideCancellation to true, inconsistent with the constructor and fromArray() which both default to false. Corrected.
  • Removed an unnecessary new SplQueue() allocation in releaseClean() that replaced the existing queue object when it was already empty.
  • Removed a redundant $this->activeConnections = 0 in checkShutdownComplete(). Active connections are already zero at that point.
  • Renamed the $MysqlConfig property on PoolManager to $mysqlConfig to match PSR naming.
  • Removed a duplicate docblock above createNewConnection().
  • Extracted the repeated queue-drain loop in healthCheck() into a single closure shared by both the resolve and reject callbacks.
  • Added a unit test covering the fromUri() default correction.

@jeffreybulanadi jeffreybulanadi requested a review from rcalicdan May 14, 2026 04:02
@jeffreybulanadi jeffreybulanadi changed the title Pool manager and config cleanup from issue 5 Rename MysqlConfig property, correct fromUri default, and remove redundant code May 14, 2026

@rcalicdan rcalicdan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the refactor and cleaning up the codebase..

@rcalicdan rcalicdan merged commit 806633f into hiblaphp:main May 14, 2026
6 checks passed
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.

Improvements identified in codebase review

2 participants