Skip to content

Conversation

@gtcooke94
Copy link
Contributor

Add documentation to the main package, the RootCertificateOptions struct, and the IdentityCertificateOptions struct.

RELEASE NOTES: none

@gtcooke94 gtcooke94 added the Type: Documentation Documentation or examples label May 8, 2024
@gtcooke94 gtcooke94 added this to the 1.64 Release milestone May 8, 2024
@gtcooke94 gtcooke94 requested a review from erm-g May 8, 2024 17:24
@gtcooke94 gtcooke94 requested a review from dfawley May 8, 2024 17:26
@gtcooke94 gtcooke94 assigned dfawley and unassigned erm-g May 8, 2024
@gtcooke94 gtcooke94 removed the request for review from erm-g May 8, 2024 17:27
@codecov
Copy link

codecov bot commented May 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.55%. Comparing base (adf976b) to head (2e37a56).
Report is 51 commits behind head on master.

❗ Current head 2e37a56 differs from pull request most recent head d7aff45. Consider uploading reports for the commit d7aff45 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7213      +/-   ##
==========================================
- Coverage   81.24%   80.55%   -0.69%     
==========================================
  Files         345      349       +4     
  Lines       33941    34040      +99     
==========================================
- Hits        27574    27421     -153     
- Misses       5202     5428     +226     
- Partials     1165     1191      +26     

see 32 files with indirect coverage changes

@arvindbr8 arvindbr8 modified the milestones: 1.64 Release, 1.65 Release May 9, 2024
// Package advancedtls is a utility library containing functions to construct
// credentials.TransportCredentials that can perform credential reloading and
// custom verification check.
// Package advancedtls is a library containing APIs for configuring grpc
Copy link
Member

Choose a reason for hiding this comment

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

Nit: all packages are libraries, and all libraries contain APIs, so I think you could remove some words here to make everything more to-the-point. How about something like:

Package advancedtls provides gRPC transport credentials that allow easy configuration of advanced TLS features.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed, I like that

// for both the client and the server.
// At most one option could be set. If none of them are set, we
// use the system default trust certificates.
// At most one option should be set. If none of them are set, we use the system
Copy link
Member

Choose a reason for hiding this comment

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

What is an "option"? Is each option a single field in this struct? If so, maybe "at most one field should be set"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed, good call


// IdentityCertificateOptions contains options to obtain identity certificates
// for both the client and the server.
// At most one option could be set.
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.

Changed

@dfawley dfawley assigned gtcooke94 and unassigned dfawley May 13, 2024
@gtcooke94 gtcooke94 requested a review from dfawley May 13, 2024 17:43
@gtcooke94 gtcooke94 assigned dfawley and unassigned gtcooke94 May 13, 2024
@dfawley dfawley assigned gtcooke94 and unassigned dfawley May 13, 2024
@gtcooke94 gtcooke94 merged commit 0020ccf into grpc:master May 13, 2024
1 check passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Type: Documentation Documentation or examples

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants