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

Use function-style tags for options #78

Merged

Conversation

thockin
Copy link
Collaborator

@thockin thockin commented Dec 18, 2024

This defines a parser for tags which takes a single Go-style identifier argument (but leaves room for better parsing).

@thockin thockin force-pushed the validation-gen-th-functional-tags branch from 0fecd2d to 291bd6a Compare December 18, 2024 23:18
@thockin thockin changed the title DNM: Use funtional tags for options DNM: Use function-style tags for options Dec 19, 2024
@thockin thockin force-pushed the validation-gen-th-functional-tags branch 4 times, most recently from 7e85a69 to fde6b4f Compare December 19, 2024 19:58
@thockin thockin force-pushed the validation-gen-th-functional-tags branch from fde6b4f to 2175c02 Compare January 7, 2025 00:23
@thockin thockin changed the title DNM: Use function-style tags for options Use function-style tags for options Jan 7, 2025
@thockin thockin force-pushed the validation-gen-th-functional-tags branch from 2175c02 to 30c8385 Compare January 7, 2025 00:25
@thockin
Copy link
Collaborator Author

thockin commented Jan 7, 2025

Gengo is merged and vendor is updated here - ready for review.

@yongruilin
Copy link
Collaborator

yongruilin commented Jan 7, 2025

Could you rebase to include the github action change to let the CI run correctly?

Separate discussion: Did we think of having this work with compatibility version? e.g. +k8s:ifOptionsEnabled(featureX,1.23)=xxx

This defines a parser for tags which takes a single Go-style identifier
argument (but leaves room for better parsing).
@thockin thockin force-pushed the validation-gen-th-functional-tags branch from 30c8385 to 5054d5a Compare January 7, 2025 06:01
@thockin
Copy link
Collaborator Author

thockin commented Jan 7, 2025

Did we think of having this work with compatibility version? e.g. +k8s:ifOptionsEnabled(featureX,1.23)=xxx

what would the semantics of that be?

@thockin
Copy link
Collaborator Author

thockin commented Jan 7, 2025

new push is up with rebase

@yongruilin
Copy link
Collaborator

LGTM

Did we think of having this work with compatibility version? e.g. +k8s:ifOptionsEnabled(featureX,1.23)=xxx

what would the semantics of that be?

I was thinking that this means the featureX was introduce in 1.23, when emulated version is less than 1.23 (e.g. 1.22/1.21), this validation would skip.

@yongruilin yongruilin merged commit 3ad844b into jpbetz:validation-gen Jan 7, 2025
1 check passed
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