On Wed, Sep 28, 2016 at 5:12 AM, Gunnar Morling <gunnar@hibernate.org> wrote:
> * What are the shortcomings meant by "2.1.1. Working around limitations in
> ConstraintValidator that prevent the above strategy"?

All those sections without content are still to be written. In this
case, according to the spec, there is no way to write a
PastTemporalAccessorComparableConstraintValidator extends
ConstraintValidator<Past, ? extends TemporalAccessor & Comparable<?>>
or something like it, which is the natural choice. Making it just
"extends ConstraintValidator<TemporalAccessor>" won't make it fail at
the right phase if @Past is applied to a non-Comparable
TemporalAccessor. I have a few ideas on how to address that, but maybe
now you can get thinking too :-)

Ok, got you.

That'd only be a problem though if you wanted to write a single validator implementation for all of them, right? I'm wondering whether one wouldn't use several more specific implementations anyways, given one needs to invoke a specific static now() method?

Ideally we'd want to use a single implementation since it automatically add supports for any custom TemporalAccessor out there, including ThreeTen-Extra or project specific ones (I've written a few of them).

I've listed a few available at ThreeTen-Extra in the version I've sent two days ago:

https://github.com/sjmisterm/beanvalidation.org/blob/patch-1/proposals/BVAL-496.adoc
 
 
> * What do you have in mind with regards to "2.4. "Simple" TemporalAmount
> implementations support"?

These are TemporalAmount(s) with a single unit. For these, @Max and
@Min support should work for the single unit they support.

Ah, seeing now that Duration and Period implement these. I don't think I like @Max/@Min for these as it's not clear what the unit of the expected value is.

Duration and Period are *not* TemporalAmount(s) with a single unit. I was talking about classes such as:

http://www.threeten.org/threeten-extra/apidocs/org/threeten/extra/Days.html
 
New constraints may be more useful here:

    public void save(@MaxDuration(value=60, unit=ChronoUnit.SECONDS) Duration d) { ... }

Exactly, even though we should name it around TemporalAmount or ChronoUnit :-)

> * Regarding Duration/Period, how about handling this separately? It seems we
> could add this in a second step, allowing to align quickly on the
> @Future/@Past additions and add support for it to the RI.

Your call :-) I was just trying to consolidate everything about
JSR-310 in a single proposal; there's nothing wrong with splitting it.

Ok, I'd say if think you can come up with something for this quickly, go for it. Otherwise let's keep it separately. It'd be nice to have a first cut of stuff we can take and provide support for in the RI.

Let's see how much we can evolve this week.
 
Thanks a lot for your effort, Michael, that's much appreciated!

That's what I've signed up for ;-)

Regards,
Michael