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

HV-860 Using shared validateConstraint() method for property/value valid... #299

Merged
merged 1 commit into from Jan 22, 2014

Conversation

Projects
None yet
3 participants
@gunnarmorling
Copy link
Member

gunnarmorling commented Jan 22, 2014

...ation to make sure any set unwrapper is used

HV-860 Using shared validateConstraint() method for property/value va…
…lidation to make sure any set unwrapper is used
@cloudbees-pull-request-builder

This comment has been minimized.

Copy link

cloudbees-pull-request-builder commented Jan 22, 2014

HV-5-MASTER #412 SUCCESS
This pull request looks good

@@ -498,6 +498,7 @@ private void validateConstraintsForNonDefaultGroup(ValidationContext<?> validati

private boolean validateConstraint(ValidationContext<?> validationContext,
ValueContext<?, Object> valueContext,
boolean propertyPathComplete,

This comment has been minimized.

Copy link
@hferentschik

hferentschik Jan 22, 2014

Member

OOI, why did you put the parameter in the middle? I would have put last.

This comment has been minimized.

Copy link
@gunnarmorling

gunnarmorling Jan 22, 2014

Author Member

Good question; I think I considered the first parameters as stuff which forms the "context" that is needed to validate the given constraint, so I liked them to be together.

This comment has been minimized.

Copy link
@hferentschik

hferentschik Jan 22, 2014

Member

Ok. For some reason, I dislike having boolean in the middle of the parameter list. I rather have them at the end. Not really sure why. Feels better!?

return validationContext.getFailingConstraints()
.size() - numberOfConstraintViolationsBefore;
}
validateConstraint( validationContext, valueContext, true, metaConstraint );

This comment has been minimized.

Copy link
@hferentschik

hferentschik Jan 22, 2014

Member

come consolidation :-)

This comment has been minimized.

Copy link
@gunnarmorling

gunnarmorling Jan 22, 2014

Author Member

Yeah :) There is more that could be done... ValidatorImpl is in some kind of messy state with several redundancies.

This comment has been minimized.

Copy link
@hferentschik

hferentschik Jan 22, 2014

Member

ValidatorImpl is in some kind of messy state with several redundancies.

+1

@hferentschik hferentschik merged commit 612e0fa into hibernate:master Jan 22, 2014

@gunnarmorling gunnarmorling deleted the gunnarmorling:HV-860 branch Jan 28, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.