-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Add package documentation to certain files #55
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 7 files reviewed, 7 unresolved discussions (waiting on @matthewstevenson88 and @Ryanfsdf)
security/s2a/s2a.go, line 20 at r1 (raw file):
/* Package s2a provides the S2A transport credentials used by gRPC.
s/gRPC/a gRPC application
security/s2a/internal/authinfo/authinfo.go, line 21 at r1 (raw file):
/* Package authinfo provides authentication and authorization information from the S2A session result to the gRPC stack.
s/from the S2A session result/that results from the TLS handshake
security/s2a/internal/fakehandshaker/main.go, line 19 at r1 (raw file):
*/ // The fakehandshaker binary provides a fake handshaker service for S2A.
s/fake handshaker service for S2A/fake S2A handshaker service
security/s2a/internal/handshaker/handshaker.go, line 19 at r1 (raw file):
*/ // Package handshaker implements the S2A handshaker service.
s/implements/communicates with
security/s2a/internal/record/record.go, line 19 at r1 (raw file):
*/ // Package record implements the TLS 1.3 record protocol.
Please add ...used by the S2A transport credentials.
at the end
security/s2a/internal/record/internal/aeadcrypter/aeadcrypter.go, line 21 at r1 (raw file):
/* Package aeadcrypter provides the interface for AEAD Crypter implementations used by S2A.
s/provides/is
s/AEAD Crypter/the AEAD cipher
s/S2A/S2A's record protocol
security/s2a/internal/record/internal/halfconn/halfconn.go, line 20 at r1 (raw file):
/* Package halfconn implements a half connection for managing one side of a TLS 1.3
Suggested edit:
Package halfconn manages the inbound or outbound traffic of a TLS 1.3 connection.
3adc2d2
to
38f8733
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 7 files reviewed, 7 unresolved discussions (waiting on @matthewstevenson88)
security/s2a/s2a.go, line 20 at r1 (raw file):
Previously, matthewstevenson88 wrote…
s/gRPC/a gRPC application
Done.
security/s2a/internal/authinfo/authinfo.go, line 21 at r1 (raw file):
Previously, matthewstevenson88 wrote…
s/from the S2A session result/that results from the TLS handshake
Done.
security/s2a/internal/fakehandshaker/main.go, line 19 at r1 (raw file):
Previously, matthewstevenson88 wrote…
s/fake handshaker service for S2A/fake S2A handshaker service
Done.
security/s2a/internal/handshaker/handshaker.go, line 19 at r1 (raw file):
Previously, matthewstevenson88 wrote…
s/implements/communicates with
Oh oops, my initial comment isn't correct at all.
security/s2a/internal/record/record.go, line 19 at r1 (raw file):
Previously, matthewstevenson88 wrote…
Please add
...used by the S2A transport credentials.
at the end
Done.
security/s2a/internal/record/internal/aeadcrypter/aeadcrypter.go, line 21 at r1 (raw file):
Previously, matthewstevenson88 wrote…
s/provides/is
s/AEAD Crypter/the AEAD cipher
s/S2A/S2A's record protocol
Done, minus the first suggestion. I don't think it would be good to say that Package aeadcrypter is the interface
, I think it may be better to say that it provides an interface, since the thing that is the interface is the interface defined within the package rather than the package itself. WDYT?
security/s2a/internal/record/internal/halfconn/halfconn.go, line 20 at r1 (raw file):
Previously, matthewstevenson88 wrote…
Suggested edit:
Package halfconn manages the inbound or outbound traffic of a TLS 1.3 connection.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 7 files reviewed, all discussions resolved
@@ -16,6 +16,10 @@ | |||
* | |||
*/ | |||
|
|||
/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use the same format everywhere, //
instead of /* */
.
38f8733
to
8e33809
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 7 files reviewed, 1 unresolved discussion (waiting on @cesarghali)
security/s2a/internal/authinfo/authinfo.go, line 19 at r2 (raw file):
Previously, cesarghali (Cesar Ghali) wrote…
Let's use the same format everywhere,
//
instead of/* */
.
Done.
8e33809
to
5909470
Compare
Added package documentation to certain key files. This should give a very brief overview of what each of these packages are responsible for.
This change is