Skip to content

Conversation

dfawley
Copy link
Member

@dfawley dfawley commented Mar 21, 2025

This is #8187 but for EndpointMap. In this case, I could find zero external usages of this type, since it is so new, so I thought it would be OK to just convert it in place.

RELEASE NOTES:

  • resolver: convert EndpointMap to use generics

@dfawley dfawley added the Type: API Change Breaking API changes (experimental APIs only!) label Mar 21, 2025
@dfawley dfawley added this to the 1.72 Release milestone Mar 21, 2025
@dfawley dfawley requested a review from arjan-bal March 21, 2025 20:55
Copy link

codecov bot commented Mar 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.93%. Comparing base (b0d1203) to head (bb928ec).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8189      +/-   ##
==========================================
- Coverage   82.06%   81.93%   -0.14%     
==========================================
  Files         410      410              
  Lines       40233    40216      -17     
==========================================
- Hits        33018    32949      -69     
- Misses       5854     5892      +38     
- Partials     1361     1375      +14     
Files with missing lines Coverage Δ
balancer/endpointsharding/endpointsharding.go 93.10% <100.00%> (-0.08%) ⬇️
balancer/leastrequest/leastrequest.go 88.49% <100.00%> (-0.21%) ⬇️
balancer/weightedroundrobin/balancer.go 83.23% <100.00%> (-0.19%) ⬇️
resolver/map.go 94.39% <100.00%> (ø)
xds/internal/balancer/outlierdetection/balancer.go 89.36% <100.00%> (+0.95%) ⬆️
xds/internal/balancer/ringhash/ring.go 92.85% <100.00%> (-0.11%) ⬇️
xds/internal/balancer/ringhash/ringhash.go 93.86% <100.00%> (-0.06%) ⬇️

... and 18 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@arjan-bal arjan-bal left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -174,7 +174,7 @@ func (s) TestAddressMap_Values(t *testing.T) {
}

func (s) TestEndpointMap_Length(t *testing.T) {
em := NewEndpointMap()
em := NewEndpointMap[any]()
Copy link
Contributor

Choose a reason for hiding this comment

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

We can use struct{} as a more precise type parameter here, and int in the test cases below. Any reason to use any?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. I just did the simplest change possible for the tests, but using the proper type is better.

@arjan-bal arjan-bal assigned dfawley and unassigned arjan-bal Mar 24, 2025
@dfawley dfawley assigned arjan-bal and unassigned dfawley Mar 24, 2025
@dfawley dfawley merged commit a51009d into grpc:master Mar 24, 2025
13 checks passed
@dfawley dfawley deleted the EndpointMapGenerics branch March 24, 2025 16:37
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Apr 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: API Change Breaking API changes (experimental APIs only!)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants