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

docs: rework current repo docs #520

Merged
merged 1 commit into from
Dec 26, 2021
Merged

docs: rework current repo docs #520

merged 1 commit into from
Dec 26, 2021

Conversation

alecholmez
Copy link
Contributor

Signed-off-by: Alec Holmes alecholmez@me.com

closes #513

Signed-off-by: Alec Holmes <alecholmez@me.com>
@alecholmez
Copy link
Contributor Author

@snowp just a quick fix up for docs here

@jpeach
Copy link
Contributor

jpeach commented Dec 3, 2021

@alecholmez WDYT about moving this to the doc.go in the relevant packages, and moving the code snippets into testable examples?

@alecholmez
Copy link
Contributor Author

@jpeach I think that would be a great idea, it looks like most people are using those so maybe we can push the changes through but then also do that? Honestly I don't like these docs since they require updating often especially if we make API changes. IMO looking at the tests are better anyways :) but I do allude to the code example quite a lot to minimize how much I need to change these docs

Copy link
Contributor

@jpeach jpeach left a comment

Choose a reason for hiding this comment

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

This looks pretty reasonable to me.

@alecholmez
Copy link
Contributor Author

@snowp can you give the stamp of approval over here? It looks like James doesn't have repo control yet so his approval didn't allow a merge

The following guides may be helpful on how to use go-control-plane's Snapshot Cache:
- [Snapshot.md](cache/Snapshot.md)
- [Server.md](cache/Server.md)
- [cache.md](snapshot.md)
Copy link
Member

Choose a reason for hiding this comment

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

nit: snapshot.md as link text?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah oops thank you, it should be cache.md

Copy link
Member

Choose a reason for hiding this comment

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

I think this was merged without updating this change, submitted #535

@alecholmez alecholmez merged commit 1bbf44f into main Dec 26, 2021
@alecholmez alecholmez deleted the docs branch December 26, 2021 03:56
sunjayBhatia added a commit to sunjayBhatia/go-control-plane that referenced this pull request Jan 3, 2022
From envoyproxy#520

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
@sunjayBhatia sunjayBhatia mentioned this pull request Jan 3, 2022
alecholmez pushed a commit that referenced this pull request Jan 3, 2022
From #520

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Help with cache.NewSnapshot breaking change and outdated documentation
4 participants