Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

internal/transport: Remove redundant if conditional in http2_server #6946

Merged
merged 1 commit into from
Jan 29, 2024

Conversation

chengfang
Copy link
Contributor

@chengfang chengfang commented Jan 29, 2024

In internal/transport/http2_server.go, around line 640, the if-else branches contain the identical code, and should be combined:

if err == io.EOF || err == io.ErrUnexpectedEOF {
  t.Close(err)
  return
}
t.Close(err)
return

RELEASE NOTES: none

Copy link

linux-foundation-easycla bot commented Jan 29, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: chengfang / name: Cheng Fang (b8b6bfb)

Copy link

codecov bot commented Jan 29, 2024

Codecov Report

Merging #6946 (b8b6bfb) into master (5051eea) will decrease coverage by 0.21%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6946      +/-   ##
==========================================
- Coverage   83.81%   83.61%   -0.21%     
==========================================
  Files         287      287              
  Lines       30930    30926       -4     
==========================================
- Hits        25925    25858      -67     
- Misses       3952     3999      +47     
- Partials     1053     1069      +16     
Files Coverage Δ
internal/transport/http2_server.go 89.56% <ø> (-0.04%) ⬇️

... and 21 files with indirect coverage 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. @zasweq -- do you mind taking a second pass at this?

@arvindbr8 arvindbr8 requested a review from zasweq January 29, 2024 22:28
@arvindbr8 arvindbr8 added the Type: Internal Cleanup Refactors, etc label Jan 29, 2024
@arvindbr8 arvindbr8 added this to the 1.62 Release milestone Jan 29, 2024
@zasweq zasweq changed the title internal/transport: Combine the identical if-else branches in internal/transport/http2_server.go internal/transport: Combine the identical if-else branches in http2_server Jan 29, 2024
@zasweq zasweq changed the title internal/transport: Combine the identical if-else branches in http2_server internal/transport: Remove redundant if conditional in http2_server Jan 29, 2024
@zasweq zasweq merged commit 8d735f0 into grpc:master Jan 29, 2024
14 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 28, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants