Skip to content

Conversation

@scop
Copy link
Contributor

@scop scop commented May 9, 2023

Noticed while looking into something else. No benchmarks/numbers done, but even if this wouldn't yield measurable improvements, the changes don't hurt readability IMO and actually in a sense they convey the intent better.

@marckhouzam
Copy link
Collaborator

Thanks @scop !
I haven't had as much time as I would have like for Cobra recently but things should improve in a couple of weeks. I'll give this some attention then.

@marckhouzam marckhouzam added this to the 1.8.0 milestone Jun 12, 2023
scop added 6 commits October 5, 2023 22:04
There likely isn't actually more than once to split in the source
strings in these cases, but avoid doing so anyway as we're only
interested in the first.
The target is not to be changed while outputting completions, so resolve
it only once.
The enablement state is not to be changed during completion output, so
evaluate it only once.
@scop
Copy link
Contributor Author

scop commented Oct 5, 2023

Rebased and added some more.

Copy link
Collaborator

@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
But I’m not near a computer right now so I’ll wait until I can test it to merge.

@jpmcb if you’re comfortable merging, go right ahead

@marckhouzam marckhouzam added kind/cleanup General cleanup of code, issues, etc. lgtm Denotes "looks good to me" from maintainers and signals other collaboratores that a PR is ready labels Nov 4, 2023
@marckhouzam marckhouzam merged commit 3d8ac43 into spf13:main Nov 23, 2023
@scop scop deleted the perf/micro-opt branch November 26, 2023 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/cleanup General cleanup of code, issues, etc. lgtm Denotes "looks good to me" from maintainers and signals other collaboratores that a PR is ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants