Skip to content

cleanup: remove unused constants in generic xdsclient #8315

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 3 commits into from
May 14, 2025
Merged

cleanup: remove unused constants in generic xdsclient #8315

merged 3 commits into from
May 14, 2025

Conversation

purnesh42H
Copy link
Contributor

RELEASE NOTES: None

@purnesh42H purnesh42H requested a review from dfawley May 13, 2025 11:53
@purnesh42H purnesh42H added the Type: Internal Cleanup Refactors, etc label May 13, 2025
@purnesh42H purnesh42H added this to the 1.73 Release milestone May 13, 2025
dfawley
dfawley reviewed May 13, 2025
View reviewed changes
CONTRIBUTING.md Outdated
@@ -87,7 +87,7 @@ How to get your contributions merged smoothly and quickly:

- The grpc package should only depend on standard Go packages and a small number
of exceptions. **If your contribution introduces new dependencies**, you will
need a discussion with gRPC-Go maintainers. A github action check will run on
need a discussion with gRPC-Go maintainers. A Github action check will run on
Copy link
Member

Choose a reason for hiding this comment

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

Apparently we have a special linter that will want this to be written as GitHub everywhere.

Suggested change
need a discussion with gRPC-Go maintainers. A Github action check will run on
need a discussion with gRPC-Go maintainers. A GitHub action check will run on

xds/internal/clients/xdsclient/xdsclient.go Outdated
@@ -221,7 +216,7 @@ func (c *XDSClient) Close() {
// A non-nil error is returned if an xdsChannel was not created.
func (c *XDSClient) getChannelForADS(serverConfig *ServerConfig, callingAuthority *authority) (*xdsChannel, func(), error) {
if c.done.HasFired() {
return nil, nil, ErrClientClosed
return nil, nil, errClientClosed
Copy link
Member

Choose a reason for hiding this comment

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

Given that it's used exactly once, let's just inline it.

The comment on it became stale anyway.

@dfawley dfawley assigned purnesh42H and unassigned dfawley May 13, 2025
@purnesh42H purnesh42H assigned dfawley and unassigned purnesh42H May 13, 2025
@purnesh42H purnesh42H requested a review from dfawley May 13, 2025 15:56
dfawley
dfawley approved these changes May 13, 2025
View reviewed changes
Copy link
Member

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

In future, please avoid combining unrelated changes like this.

https://github.com/grpc/grpc-go/blob/master/CONTRIBUTING.md#guidelines-for-pull-requests

  • Create small PRs that are narrowly focused on addressing a single concern.

If you wouldn't mind splitting this one up, that would be great, too.

@codecov Codecov
Copy link

codecov bot commented May 13, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.17%. Comparing base (d36b02e) to head (fc82fea).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
xds/internal/clients/xdsclient/xdsclient.go 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8315      +/-   ##
==========================================
- Coverage   82.19%   82.17%   -0.03%     
==========================================
  Files         419      419              
  Lines       41992    41990       -2     
==========================================
- Hits        34516    34504      -12     
- Misses       6008     6016       +8     
- Partials     1468     1470       +2
Files with missing lines Coverage Δ
xds/internal/clients/xdsclient/xdsclient.go 80.45% <0.00%> (+1.36%) ⬆️

... and 17 files with indirect coverage changes

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

@purnesh42H purnesh42H changed the title cleanup: remove unused constants in generic xdsclient and fix contributing.md lint issue cleanup: remove unused constants in generic xdsclient May 13, 2025
@purnesh42H
Copy link
Contributor Author

In future, please avoid combining unrelated changes like this.

https://github.com/grpc/grpc-go/blob/master/CONTRIBUTING.md#guidelines-for-pull-requests

  • Create small PRs that are narrowly focused on addressing a single concern.

If you wouldn't mind splitting this one up, that would be great, too.

I have remove the contributing.md change from this PR

@dfawley
Copy link
Member

dfawley commented May 13, 2025

Thanks!

@dfawley dfawley assigned purnesh42H and unassigned dfawley May 13, 2025
@purnesh42H purnesh42H merged commit 09166b6 into grpc:master May 14, 2025
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dfawley dfawley dfawley approved these changes

Assignees

@purnesh42H purnesh42H

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

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