Skip to content

server: Rework NewConn/defaultServer #1021

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Apr 2, 2025
Merged

server: Rework NewConn/defaultServer #1021

merged 14 commits into from
Apr 2, 2025

Conversation

dveeden
Copy link
Collaborator

@dveeden dveeden commented Apr 1, 2025

Closes #1020

  • De-duplicate NewConn() and NewCustomizedConn() code
  • Only call NewDefaultServer() when needed

@dveeden dveeden requested review from atercattus and lance6716 April 1, 2025 06:24
@dveeden dveeden mentioned this pull request Apr 1, 2025
Closed
lance6716
lance6716 reviewed Apr 1, 2025
View reviewed changes
ccoVeille
ccoVeille reviewed Apr 1, 2025
View reviewed changes
Copy link
Contributor

@ccoVeille ccoVeille left a comment

Choose a reason for hiding this comment

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

Some remarks, otherwise LGTM 👍

dveeden and others added 2 commits April 1, 2025 10:38
@dveeden @ccoVeille
Update server/conn.go
fdfe0b2 
Co-authored-by: ccoVeille <[email protected]>
@dveeden @ccoVeille
Update server/conn.go
ac35ad0 
Co-authored-by: ccoVeille <[email protected]>
rosstimothy
rosstimothy reviewed Apr 1, 2025
View reviewed changes
@dveeden @rosstimothy
Update server/conn.go
162e81d 
Co-authored-by: rosstimothy <[email protected]>
ccoVeille
ccoVeille suggested changes Apr 1, 2025
View reviewed changes
@lance6716 lance6716 mentioned this pull request Apr 2, 2025
Merged
dveeden and others added 3 commits April 2, 2025 07:33
lance6716
lance6716 reviewed Apr 2, 2025
View reviewed changes
@lance6716 lance6716 merged commit ee9447d into go-mysql-org:master Apr 2, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lance6716 lance6716 lance6716 approved these changes

@ccoVeille ccoVeille ccoVeille approved these changes

@atercattus atercattus Awaiting requested review from atercattus

@rosstimothy rosstimothy Awaiting requested review from rosstimothy

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expensive default variable creation
4 participants
@dveeden @lance6716 @ccoVeille @rosstimothy