Skip to content

credentials/alts: Add comments to clarify buffer sizing #8232

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 1 commit into from
Apr 8, 2025
Merged

credentials/alts: Add comments to clarify buffer sizing #8232

merged 1 commit into from
Apr 8, 2025

Conversation

arjan-bal
Copy link
Contributor

This addresses the following review comment: #8204 (comment)

RELEASE NOTES: N/A

@arjan-bal arjan-bal added Type: Internal Cleanup Refactors, etc Area: Auth Includes regular credentials API and implementation. Also includes advancedtls, authz, rbac etc. labels Apr 7, 2025
@arjan-bal arjan-bal added this to the 1.73 Release milestone Apr 7, 2025
@arjan-bal arjan-bal requested a review from gtcooke94 April 7, 2025 15:09
@arjan-bal arjan-bal mentioned this pull request Apr 7, 2025
Merged
@codecov Codecov
Copy link

codecov bot commented Apr 7, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.10%. Comparing base (b368379) to head (57920bf).
Report is 2 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8232      +/-   ##
==========================================
+ Coverage   81.99%   82.10%   +0.11%     
==========================================
  Files         410      410              
  Lines       40389    40394       +5     
==========================================
+ Hits        33116    33167      +51     
+ Misses       5893     5863      -30     
+ Partials     1380     1364      -16
Files with missing lines Coverage Δ
credentials/alts/internal/conn/record.go 78.20% <100.00%> (+0.72%) ⬆️

... and 26 files with indirect coverage changes

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

gtcooke94
gtcooke94 approved these changes Apr 7, 2025
View reviewed changes
Copy link
Contributor

@gtcooke94 gtcooke94 left a comment

Choose a reason for hiding this comment

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

Thanks!

@arjan-bal arjan-bal merged commit be25d96 into grpc:master Apr 8, 2025
15 checks passed
arjan-bal added a commit to arjan-bal/grpc-go that referenced this pull request Apr 8, 2025
@arjan-bal
Revert "credentials/alts: Add comments to clarify buffer sizing (grpc…
88f03d5 
…#8232)"

This reverts commit be25d96.
arjan-bal added a commit that referenced this pull request Apr 8, 2025
@arjan-bal
Rollback #8232 and #8204 (#8236)
6bfa0ca 
* Revert "credentials/alts: Add comments to clarify buffer sizing (#8232)"

This reverts commit be25d96.

* Revert "credentials/alts: Optimize reads (#8204)"

This reverts commit b368379.
purnesh42H pushed a commit to purnesh42H/grpc-go that referenced this pull request Apr 13, 2025
@arjan-bal @purnesh42H
credentials/alts: Add comments to clarify buffer sizing (grpc#8232)
6e330e9
purnesh42H pushed a commit to purnesh42H/grpc-go that referenced this pull request Apr 13, 2025
@arjan-bal @purnesh42H
Rollback grpc#8232 and grpc#8204 (grpc#8236)
ed9081b 
* Revert "credentials/alts: Add comments to clarify buffer sizing (grpc#8232)"

This reverts commit be25d96.

* Revert "credentials/alts: Optimize reads (grpc#8204)"

This reverts commit b368379.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtcooke94 gtcooke94 gtcooke94 approved these changes

Assignees

@gtcooke94 gtcooke94

Labels
Area: Auth Includes regular credentials API and implementation. Also includes advancedtls, authz, rbac etc. Type: Internal Cleanup Refactors, etc
Projects
None yet
Milestone
1.73 Release
Development

Successfully merging this pull request may close these issues.
2 participants
@arjan-bal @gtcooke94