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

Implement singleton pattern for NewRootCmd to prevent multiple creations #840

Merged
merged 3 commits into from
Jan 14, 2025

Conversation

anujc25
Copy link
Contributor

@anujc25 anujc25 commented Jan 10, 2025

What this PR does / why we need it

  • Fix pinniped plugin invocation by not creating NewRootCmd and using existing one

Which issue(s) this PR fixes

Fixes #825

Describe testing done for PR

Release note

Implement singleton pattern for `NewRootCmd` to prevent multiple creations

Additional information

Special notes for your reviewer

@anujc25 anujc25 force-pushed the anujc/fix-root-cmd-invocation branch 5 times, most recently from 6c63b90 to 07b71e5 Compare January 10, 2025 22:38
@anujc25 anujc25 changed the title Fix pinniped plugin invocation by not creating NewRootCmd and using existing one Implement singleton pattern for NewRootCmd to prevent multiple creations Jan 10, 2025
@anujc25 anujc25 force-pushed the anujc/fix-root-cmd-invocation branch from 07b71e5 to 02cc270 Compare January 10, 2025 22:42
Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

Nice solution!

pkg/command/root.go Outdated Show resolved Hide resolved
pkg/command/root.go Show resolved Hide resolved
pkg/command/root.go Show resolved Hide resolved
@anujc25 anujc25 marked this pull request as ready for review January 13, 2025 18:26
@anujc25 anujc25 requested a review from a team as a code owner January 13, 2025 18:26
Signed-off-by: Anuj Chaudhari <[email protected]>
@anujc25 anujc25 force-pushed the anujc/fix-root-cmd-invocation branch from bf4217d to 901fd27 Compare January 13, 2025 19:58
Copy link
Contributor

@marckhouzam marckhouzam 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 doing this!

Copy link
Contributor

@prkalle prkalle 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 the changes!

@anujc25 anujc25 merged commit 42cf56b into vmware-tanzu:main Jan 14, 2025
7 checks passed
@anujc25 anujc25 added this to the v1.5.2 milestone Jan 14, 2025
anujc25 added a commit to anujc25/tanzu-cli that referenced this pull request Jan 14, 2025
…ons (vmware-tanzu#840)

* Only allow creating the NewRootCmd once with singleton pattern
* Fix unit tests by using NewRootCmdForTest

Signed-off-by: Anuj Chaudhari <[email protected]>
anujc25 added a commit that referenced this pull request Jan 14, 2025
…ons (#840)

* Only allow creating the NewRootCmd once with singleton pattern
* Fix unit tests by using NewRootCmdForTest

Signed-off-by: Anuj Chaudhari <[email protected]>
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.

tanzu-cli 1.4.1 on Linux context create hangs on kubernetes-release
5 participants