Skip to content

Remove V2 PeerName field from pbresource.Tenancy#19865

Merged
mkeeler merged 1 commit intomainfrom
remove-v2-peername
Jan 29, 2024
Merged

Remove V2 PeerName field from pbresource.Tenancy#19865
mkeeler merged 1 commit intomainfrom
remove-v2-peername

Conversation

@mkeeler
Copy link
Copy Markdown
Member

@mkeeler mkeeler commented Dec 8, 2023

Description

The peer name will eventually show up elsewhere in the resource. For now though this rips it out of where we don’t want it to be.

Testing & Reproduction steps

All existing tests should pass

@mkeeler mkeeler requested a review from dhiaayachi December 8, 2023 16:05
@mkeeler mkeeler requested review from a team as code owners December 8, 2023 16:05
@github-actions github-actions bot added type/docs Documentation needs to be created/updated/clarified theme/cli Flags and documentation for the CLI interface theme/envoy/xds Related to Envoy support labels Dec 8, 2023
@mkeeler mkeeler added pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport labels Dec 8, 2023
Copy link
Copy Markdown
Contributor

@boruszak boruszak left a comment

Choose a reason for hiding this comment

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

From the docs perspective, there's only one page edited, so I'm approving with a LGTM so you're not blocked.

BUT: that page lists a -peer flag for the command. Does this flag still work after removing the PeerName field? If not we should remove that flag's listing on the page as well.

@mkeeler mkeeler force-pushed the remove-v2-peername branch from 0475e96 to fdeb521 Compare December 8, 2023 17:07
@mkeeler
Copy link
Copy Markdown
Member Author

mkeeler commented Dec 8, 2023

@boruszak I went ahead and removed other mentions of the -peer cli command from the docs.

Copy link
Copy Markdown
Contributor

@dhiaayachi dhiaayachi left a comment

Choose a reason for hiding this comment

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

LGTM!
@mkeeler I resolved the conflicts on this, I hope that's fine 😅
I think we should merge this ASAP before it conflict again

The peer name will eventually show up elsewhere in the resource. For now though this rips it out of where we don’t want it to be.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr/no-backport pr/no-changelog PR does not need a corresponding .changelog entry theme/cli Flags and documentation for the CLI interface theme/envoy/xds Related to Envoy support type/docs Documentation needs to be created/updated/clarified

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants