Skip to content
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

client: simplify initialization and cleanup a bit #6798

Merged
merged 6 commits into from
Nov 15, 2023
Merged

client: simplify initialization and cleanup a bit #6798

merged 6 commits into from
Nov 15, 2023

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented Nov 15, 2023 •
edited by arvindbr8
Loading

  • Unconditionally initialize blockingpicker (renamed to pickerWrapper) and balancerWrapper to simplify cleanup.
  • Rearrange things in DialContext a bit further to facilitate the above
  • Access mutex-protected fields in Close after Unlock directly, as it is safe.

RELEASE NOTES: none

@dfawley dfawley added the Type: Internal Cleanup Refactors, etc label Nov 15, 2023
@dfawley dfawley added this to the 1.61 Release milestone Nov 15, 2023
@dfawley dfawley requested a review from arvindbr8 November 15, 2023 17:28
@codecov Codecov
Copy link

codecov bot commented Nov 15, 2023 •
edited
Loading

Codecov Report

Merging #6798 (cab5a88) into master (424db25) will increase coverage by 0.03%.
Report is 1 commits behind head on master.
The diff coverage is 92.00%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #6798      +/-   ##
==========================================
+ Coverage   83.50%   83.54%   +0.03%     
==========================================
  Files         285      285              
  Lines       30969    30946      -23     
==========================================
- Hits        25860    25853       -7     
+ Misses       4035     4026       -9     
+ Partials     1074     1067       -7
Files Coverage Δ
balancer_conn_wrappers.go 80.97% <100.00%> (+0.07%) ⬆️
clientconn.go 92.91% <90.00%> (-0.14%) ⬇️

... and 15 files with indirect coverage changes

arvindbr8
arvindbr8 approved these changes Nov 15, 2023
View reviewed changes
Copy link
Member

@arvindbr8 arvindbr8 left a comment

Choose a reason for hiding this comment

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

LGTM

clientconn.go Outdated Show resolved Hide resolved
@arvindbr8 arvindbr8 assigned dfawley and unassigned arvindbr8 Nov 15, 2023
@dfawley dfawley merged commit ce3b538 into grpc:master Nov 15, 2023
14 checks passed
@dfawley dfawley deleted the inits branch November 15, 2023 18:47
dfawley added a commit to dfawley/grpc-go that referenced this pull request Dec 7, 2023
@dfawley
client: simplify initialization and cleanup a bit (grpc#6798)
4e8bdb7
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@arvindbr8 arvindbr8 arvindbr8 approved these changes

Assignees

@dfawley dfawley

Labels
Type: Internal Cleanup Refactors, etc
Projects
None yet
Milestone
1.61 Release
Development

Successfully merging this pull request may close these issues.
2 participants
@dfawley @arvindbr8