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

Update cobra to 1.7.0 #5444

Merged
merged 2 commits into from Aug 4, 2023
Merged

Update cobra to 1.7.0 #5444

merged 2 commits into from Aug 4, 2023

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Jul 31, 2023

This release has several updates to generated completions, requiring the fixture to be updated. AFAICT, the patching that is intended to be checked by this test is still working, so I just updated the files from the just-built output.

@QuLogic QuLogic requested a review from a team as a code owner July 31, 2023 04:03
Copy link
Contributor

@chrisd8088 chrisd8088 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 the patch and the update!

If I read the changes correctly, it looks like, for Zsh, the compdef command is now included in the script, and if that's true, we should amend the manual page's example as part of this PR to take that example step out if it's no longer necessary.

It's probably also worth double-checking in all the shells that the updated scripts perform as expected. This is a new feature and so we don't have an automated way of doing that. I'll try to find some time to check over the next while, and by all means report any test results you have.

Thanks again very much!

@QuLogic
Copy link
Contributor Author

QuLogic commented Aug 2, 2023

I did test with bash, but not the others. It was not extensive of course, but I did confirm that at least something reasonable appeared when tab-completing git lfs ....

And yes, the compdef was explicitly added to allow source of zsh completions: spf13/cobra#1917

Copy link
Contributor

@chrisd8088 chrisd8088 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 great, thank you! I tested all three shells and things seem to be working nicely, as anticipated.

@chrisd8088 chrisd8088 merged commit cbd6202 into git-lfs:main Aug 4, 2023
10 checks passed
@QuLogic QuLogic deleted the cobra-update branch August 4, 2023 09:00
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.

None yet

2 participants