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

Bump spf13/cobra to 1.8.0 and adapt PersistentPreRun usage #3693

Closed
wants to merge 1 commit into from

Conversation

kke
Copy link
Contributor

@kke kke commented Nov 7, 2023

Release notes

Sourced from github.com/spf13/cobra's releases.

v1.8.0

✨ Features

🐛 Bug fixes

🔧 Maintenance

🧪 Testing & CI/CD

✏️ Documentation


Thank you everyone who contributed to this release and all your hard work! Cobra and this community would never be possible without all of you!!!! 🐍

Full Changelog: spf13/cobra@v1.7.0...v1.8.0

Commits
  • a0a6ae0 Improve API to get flag completion function (#2063)
  • 890302a Support usage as plugin for tools like kubectl (#2018)
  • 48cea5c build(deps): bump actions/checkout from 3 to 4 (#2028)
  • 22953d8 Replace all non-alphanumerics in active help env var program prefix (#1940)
  • 00b68a1 Add tests for flag completion registration (#2053)
  • b711e87 Don't complete --help flag when flag parsing disabled (#2061)
  • 8b1eba4 Fix linter errors (#2052)
  • 4cafa37 Allow running persistent run hooks of all parents (#2044)
  • 5c962a2 build(deps): bump github.com/cpuguy83/go-md2man/v2 from 2.0.2 to 2.0.3 (#2047)
  • efe8fa3 build(deps): bump actions/setup-go from 3 to 4 (#1934)
  • Additional commits viewable in compare view

@twz123
Copy link
Member

twz123 commented Nov 7, 2023

Nice. There's also spf13/cobra#2018 which enables better plugin support. I think I remember that we had some workarounds for that that we might be able to drop now, too.

@kke kke marked this pull request as ready for review November 7, 2023 09:22
@kke kke requested a review from a team as a code owner November 7, 2023 09:22
@kke kke requested review from makhov and juanluisvaladas November 7, 2023 09:22
@twz123
Copy link
Member

twz123 commented Nov 8, 2023

I'm pondering if we can use that new functionality verbatim. IIUC the new upstream solution calls all the PersistentPreRun hooks, starting with the root until it reaches the actual command. We do have some places in which the PersistentPreRun of the actual command wants to perform some actions before the parent chain is called, and we did it in the reverse order, calling all the parents until we reached the root. This might not be a problem at all for most commands, but I think we need to give it some thought. Especially for the embedded kubectl command, which has special logic that bypasses all the PersistentPreRun handlers when plugins are called, and also performs log initialization before any PersistentPreRun handlers are invoked.

Copy link
Contributor

github-actions bot commented Dec 8, 2023

The PR is marked as stale since no activity has been recorded in 30 days

@github-actions github-actions bot added Stale and removed Stale labels Dec 8, 2023
Copy link
Contributor

github-actions bot commented Jan 8, 2024

The PR is marked as stale since no activity has been recorded in 30 days

Copy link
Contributor

This pull request has merge conflicts that need to be resolved.

Copy link
Contributor

The PR is marked as stale since no activity has been recorded in 30 days

@github-actions github-actions bot added Stale and removed Stale labels Feb 16, 2024
Copy link
Contributor

The PR is marked as stale since no activity has been recorded in 30 days

@github-actions github-actions bot added Stale and removed Stale labels Mar 18, 2024
Copy link
Contributor

The PR is marked as stale since no activity has been recorded in 30 days

@github-actions github-actions bot added the Stale label Apr 18, 2024
Copy link
Contributor

The PR is marked as stale since no activity has been recorded in 30 days

@github-actions github-actions bot added the Stale label May 20, 2024
@github-actions github-actions bot closed this May 27, 2024
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.

2 participants