Skip to content

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
@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.

2 participants