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

advancedtls: Rename VType #7149

Merged
merged 4 commits into from
Apr 19, 2024
Merged

advancedtls: Rename VType #7149

merged 4 commits into from
Apr 19, 2024

Conversation

gtcooke94
Copy link
Contributor

@gtcooke94 gtcooke94 commented Apr 18, 2024

This PR renames VType to VerificationType for clarity.
It marks VType as deprecated. For now, if VType is explicitly set (does not have the default value) then we set VerificationType to VType so that user behavior doesn't break.

RELEASE NOTES: none

@gtcooke94 gtcooke94 requested a review from erm-g April 18, 2024 17:23
@gtcooke94 gtcooke94 added Type: Internal Cleanup Refactors, etc Type: Security A bug or other problem affecting security labels Apr 18, 2024
@gtcooke94 gtcooke94 added this to the 1.64 Release milestone Apr 18, 2024
Copy link
Contributor

@erm-g erm-g left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines 182 to 183
// DEPRECATED: Renamed to `VerificationType`
// VType is the verification type on the client side.
Copy link
Member

Choose a reason for hiding this comment

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

The proper way to docstring this is:

// VType is blah blah
//
// Deprecated: use VerificationType instead.
VType VerificationType

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

// the `VerificationType` enum for the different options.
// Default: CertAndHostVerification
VerificationType VerificationType
// DEPRECATED: Renamed to `VerificationType`
Copy link
Member

Choose a reason for hiding this comment

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

As above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@dfawley dfawley assigned gtcooke94 and unassigned dfawley and erm-g Apr 19, 2024
@gtcooke94 gtcooke94 requested a review from dfawley April 19, 2024 17:39
@gtcooke94 gtcooke94 assigned dfawley and unassigned gtcooke94 Apr 19, 2024
@dfawley dfawley assigned gtcooke94 and unassigned dfawley Apr 19, 2024
@dfawley dfawley removed the Type: Internal Cleanup Refactors, etc label Apr 19, 2024
@gtcooke94 gtcooke94 merged commit 5fe2e74 into grpc:master Apr 19, 2024
12 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 17, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
Type: Security A bug or other problem affecting security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants