Skip to content
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

Merged
merged 1 commit into from
Jul 30, 2020

Conversation

Ryanfsdf
Copy link
Collaborator

@Ryanfsdf Ryanfsdf commented Jul 22, 2020

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 Reviewable

Copy link
Owner

@matthewstevenson88 matthewstevenson88 left a 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.

Copy link
Collaborator Author

@Ryanfsdf Ryanfsdf left a 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.

Copy link
Owner

@matthewstevenson88 matthewstevenson88 left a 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

@Ryanfsdf Ryanfsdf requested a review from cesarghali July 23, 2020 01:13
@@ -16,6 +16,10 @@
*
*/

/*
Copy link
Collaborator

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 /* */.

Copy link
Collaborator Author

@Ryanfsdf Ryanfsdf left a 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.

@Ryanfsdf Ryanfsdf merged commit 565b790 into matthewstevenson88:master Jul 30, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants