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

BGP CP: Adds Intro to Docs #26195

Merged
merged 1 commit into from Jun 20, 2023
Merged

BGP CP: Adds Intro to Docs #26195

merged 1 commit into from Jun 20, 2023

Conversation

danehans
Copy link
Contributor

@danehans danehans commented Jun 13, 2023

Updates BGP CP docs with an introduction section that provides an explanation of the feature.

Relates to: #26146

@danehans danehans requested review from a team as code owners June 13, 2023 19:08
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 13, 2023
@danehans
Copy link
Contributor Author

cc: @YutaroHayakawa since you commented on the related issue.

Copy link
Contributor

@harsimran-pabla harsimran-pabla left a comment

Choose a reason for hiding this comment

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

Thanks for adding this section, minor comments.

Documentation/network/bgp-control-plane.rst Outdated Show resolved Hide resolved
Documentation/network/bgp-control-plane.rst Outdated Show resolved Hide resolved
pkg/bgpv1/README.md Outdated Show resolved Hide resolved
@joestringer joestringer added the release-note/misc This PR makes changes that have no direct user impact. label Jun 14, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 14, 2023
Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

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

@danehans Good start. Words like "this" make it difficult to understand what exactly you're referring to, and what specifically is taking action. Please revise then re-request review.

Documentation/network/bgp-control-plane.rst Outdated Show resolved Hide resolved
pkg/bgpv1/README.md Outdated Show resolved Hide resolved
Copy link
Member

@YutaroHayakawa YutaroHayakawa left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks for your contribution!

Copy link
Contributor

@zacharysarah zacharysarah left a comment

Choose a reason for hiding this comment

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

@danehans Thank you, great improvement! 🏆 Only some nits to fix--approving to unblock, with the understanding that changes are required prior to merge.

Documentation/network/bgp-control-plane.rst Outdated Show resolved Hide resolved
pkg/bgpv1/README.md Outdated Show resolved Hide resolved
Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>
@danehans
Copy link
Contributor Author

Commit cbdc1de resolves @zacharysarah's review comments.

@YutaroHayakawa
Copy link
Member

Reviews are in and document preview CI is passing. Since this PR only changes documents, I'll skip running full CIs.

@YutaroHayakawa YutaroHayakawa added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 19, 2023
@borkmann borkmann merged commit 1095227 into cilium:main Jun 20, 2023
50 of 52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bgp ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants