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

feat: update default templates with default and validation markers #49

Merged
merged 5 commits into from
Feb 14, 2024

Conversation

tenstad
Copy link
Contributor

@tenstad tenstad commented Aug 7, 2023

Processes kubebuilder validation and default markers, and exposes them through the fields Validation and Default. These are displayed in two new struct field columns, and validation is shown in type descriptions.

We could introduce this without updating the default template, so that those who want can start using Default and Validation without affecting existing usage.

WONTFIX

  • merging enum markers
    • when both a type and the alias type has enum validation
    • or a field and its type has enum validation
    • the current code does not merge them as in the generated CRD
  • support for XValidation marker

Example doc

image
image

@cla-checker-service
Copy link

cla-checker-service bot commented Aug 8, 2023

💚 CLA has been signed

@mowies
Copy link

mowies commented Oct 25, 2023

@tenstad @thbkrkr is there a way to move this forward and into the main release? having markers is great, but having separate fields that are directly available in the templates would be much better.

@tenstad tenstad changed the title showcase: markers draft: feat: process markers Oct 26, 2023
@tenstad tenstad changed the title draft: feat: process markers feat: process markers Oct 26, 2023
@tenstad tenstad marked this pull request as draft October 26, 2023 05:29
@tenstad
Copy link
Contributor Author

tenstad commented Oct 26, 2023

@mowies @thbkrkr I have now rebased this PR, so that we can start thinking about how we want to introduce markers in the doc. I'm not sure how to best render defaults and validations yet 🤔 The current draft with two new columns use quite a lot of space.

@michelgb
Copy link

michelgb commented Feb 3, 2024

Any plans to merge this?

@tenstad tenstad marked this pull request as ready for review February 13, 2024 10:33
@tenstad
Copy link
Contributor Author

tenstad commented Feb 13, 2024

@thbkrkr @mowies @dashanji @michelgb @RealAnna I have now made a few more tweaks and extended the tests some more.

Suggest we move XValidation and correctly merging enum markers (edgecase) outside the scope of this MR (see WONTFIX in description). This one is therefore ready for review now :)

processor/processor.go Outdated Show resolved Hide resolved
tenstad and others added 3 commits February 14, 2024 12:27
Co-authored-by: Thibault Richard <thbkrkr@users.noreply.github.com>
Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Thanks, @tenstad.

@thbkrkr thbkrkr merged commit cf959ab into elastic:master Feb 14, 2024
2 checks passed
@thbkrkr thbkrkr changed the title feat: process markers feat: update default templates with default and validation markers Feb 14, 2024
@RealAnna
Copy link

looks amazing 🚀 looking forward for the release!

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.

5 participants