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

Adding comments to cache and generic server #475

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jackstine
Copy link
Contributor

These are just comments. Please add some comments of your own if you like.

@jackstine jackstine marked this pull request as ready for review June 6, 2022 15:24
@jackstine
Copy link
Contributor Author

jackstine commented Jun 6, 2022

/kick build-bot here

@nfuden
Copy link
Contributor

nfuden commented Jun 6, 2022

nit: Some of these comments are just repeating the exact contents of the functions.
I would argue that method comments should be less implementation linked and talk more about the responsibilities of the method / what it does from a bigger picture.
If you still think that the line by line explantations of functionality (ie using a chan) are needed they can go close to the line itself.

For example with streamenvoyv3 what does it do. It creates a bidirectional stream of envoyv3 configuration which asynchronously retrieves data from the snapshot cache and serves that content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants