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

Consistent annotation names #2140

Merged
merged 1 commit into from May 18, 2024
Merged

Consistent annotation names #2140

merged 1 commit into from May 18, 2024

Conversation

nirs
Copy link
Contributor

@nirs nirs commented May 3, 2024

Add Annotation suffix to the private annotations to allow nicer code using the constants.

For example one can use the current annotation names as a temporary variable instead of unclear shortcut. Instead of this:

rag := flagsFromAnnotation(c, f, requiredAsGroup)
me := flagsFromAnnotation(c, f, mutuallyExclusive)
or := flagsFromAnnotation(c, f, oneRequired)

We can use now:

requiredAsGrop := flagsFromAnnotation(c, f, requiredAsGroupAnnotation)
mutuallyExclusive := flagsFromAnnotation(c, f, mutuallyExclusiveAnnotation)
oneRequired := flagsFromAnnotation(c, f, oneRequiredAnnotation)

Example taken from #2105.

Add `Annotation` suffix to the private annotations to allow nicer code
using the constants.

For example one can use the current annotation names as a temporary
variable instead of unclear shortcut. Instead of this:

    rag := flagsFromAnnotation(c, f, requiredAsGroup)
    me := flagsFromAnnotation(c, f, mutuallyExclusive)
    or := flagsFromAnnotation(c, f, oneRequired)

We can use now:

    requiredAsGrop := flagsFromAnnotation(c, f, requiredAsGroupAnnotation)
    mutuallyExclusive := flagsFromAnnotation(c, f, mutuallyExclusiveAnnotation)
    oneRequired := flagsFromAnnotation(c, f, oneRequiredAnnotation)

Example taken from spf13#2105.
@marckhouzam
Copy link
Collaborator

marckhouzam commented May 18, 2024

Thanks @nirs . This is fine but it will prevent #2105 from merging. So I won't merge this one right away, review #2105 first and see where we stand.

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
Since #2105 needs a new revision, we can merge this and have #2105 rebase.

@marckhouzam marckhouzam merged commit 5c2c1d6 into spf13:main May 18, 2024
18 checks passed
@marckhouzam marckhouzam added this to the 1.9.0 milestone May 18, 2024
@nirs nirs deleted the annotations branch May 18, 2024 14:17
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