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

Upgrade java, maven and versions of other dependencies #34

Merged
merged 7 commits into from
Dec 18, 2018

Conversation

velinovskav
Copy link
Collaborator

No description provided.

Valerija Velinovska added 3 commits May 4, 2018 10:35
Copy link
Collaborator

@marcelstoer marcelstoer left a comment

Choose a reason for hiding this comment

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

pom.xml Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented May 10, 2018

Coverage Status

Coverage increased (+0.8%) to 91.026% when pulling 3af21f3 on velinovskav:master into 6ae2035 on netceteragroup:master.

…ing for hibernate-validator dependency (it was relocated from org.hibernate to org.hibernate.validator)
@marcelstoer
Copy link
Collaborator

This is taking shape quite nicely. However, the more I think about it the more hesitant I become. There's a clearly defined mapping between Bean Validation constraints and valdr Angular JS validators as per https://github.com/netceteragroup/valdr-bean-validation#mapping-of-bean-validation-constraints-to-valdr-constraints (which this PR should also update). This is established through a String match from e.g. https://github.com/netceteragroup/valdr/blob/master/src/core/validators/hibernateEmailValidator.js#L12 to https://github.com/netceteragroup/valdr-bean-validation/blob/master/valdr-bean-validation/src/main/java/com/github/valdr/BuiltInConstraint.java#L33. So, if you change hibernateEmail to email you'll effectively use a different validator (the one here https://github.com/netceteragroup/valdr-bean-validation/blob/master/valdr-bean-validation/src/main/java/com/github/valdr/BuiltInConstraint.java#L33).

I'm thinking that maybe we should add support for the Bean Validation email constraint without dropping the Hibernate email support at the same time. They could basically co-exist. However, we could also argue that anyone stuck with the proprietary and deprecated Hibernate stuff should simply not upgrade this library.

What do you think?

@velinovskav
Copy link
Collaborator Author

Sorry for the delay of my response.
In my opinion it's better if there is only one validation constraint for email, if somebody wants to use Hibernate email support, should not upgrade the version of the library.
In order to be consistent I can remove mapping of Email Bean Validation constraint to valdr Email validation constraint.
What do you think?

@netceteragroupadmin netceteragroupadmin merged commit 5beb903 into netceteragroup:master Dec 18, 2018
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.

4 participants