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

Now write default annotation values to adapter Attributes #33

Merged
merged 3 commits into from
May 21, 2023

Conversation

SentryMan
Copy link
Collaborator

@SentryMan SentryMan commented May 21, 2023

now attributes should work as expected

@SentryMan SentryMan changed the title Now write default annotation values to adapters Now write default annotation values to adapter Attributes May 21, 2023
@rob-bygrave
Copy link
Contributor

Oh wow - very cool !!! member::getDefaultValue ... I didn't even look for that - nice !!

@SentryMan
Copy link
Collaborator Author

SentryMan commented May 21, 2023

The Annotation Mirror API is pretty arcane I would say, now I just gotta fix these tests

@rob-bygrave
Copy link
Contributor

rob-bygrave commented May 21, 2023

expected: "size must be between 2 and 4"
 but was: "{jakarta.validation.constraints.Size.message}"

Ah, this looks like the one default value that we didn't want it seems. Edit: But maybe we want it on custom validators in the future ...

@SentryMan
Copy link
Collaborator Author

it's possible some may want to use the same jakarta properties yeah

@rob-bygrave
Copy link
Contributor

same jakarta properties

Not really or at least this is not the case for the Jakarta validator that we support out of the box. All of those ones will use the {avaje...} resource bundle keys [except I'm also looking to tweak for DecimalMax based on the inclusive flag in order that we have NO Expression Language use in the messages / so our message interpolation is just simple token replacement only].

So for example, in BasicAdapters these are all going to translate to {avaje...} ... I could probably do the change for this I think so our tests pass.

@SentryMan
Copy link
Collaborator Author

SentryMan commented May 21, 2023

were we not going to give the ability to specify what resource bundle path to use? If we have that there is no need to modify it too much.

@SentryMan
Copy link
Collaborator Author

but feel free to modify the PR

@rbygrave
Copy link
Contributor

Ok, I'll merge this in. I intentionally left out the .validation.constraints. from the keys so I'll revert that part of the change back.

@rbygrave rbygrave merged commit 5e27f60 into avaje:main May 21, 2023
@SentryMan SentryMan deleted the defaults branch May 21, 2023 23:00
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.

3 participants