Period conversion support should not be public

[snicoll]

StringToPeriodConverter is at odds with other converters at the moment (being public and not flagged final).

2.3.0 is out so that would be a breaking change.

[snicoll]

Discussing this with @jhoeller a bit more, I think we should expand the scope a bit. I thought initially that it is possible to register more than one Converter for the same pair and return null if the input can't be handled to let another converter try. That would be an elegant solution if one had to support a slightly different format, registering theirs before ours. Turns out only one converter per pair is supported.

If our converter is not a straightforward call, we should consider:

• Making the converter public
• Moving the conversion logic to a public API so that others can reuse it in their custom implementation.

[OrangeDog]

The conversion logic in this case is almost public: PeriodStyle.detectAndParse(String, ChronoUnit).
However, that doesn't account for the @PeriodFormat or @PeriodUnit annotations that the converter also uses.

[philwebb]

I think for now we should risk making this one package private in 2.3.1. The chances of anyone extending it yet are pretty slim.

[philwebb]

If our converter is not a straightforward call, we should consider...

My feeling is that the converters are generally not designed to be subclasses or extended so we should keep them package private and not offer any public API. At least, not until someone specifically asks us to.

[snicoll]

At least, not until someone specifically asks us to.

@OrangeDog did for the duration support on Gitter which triggered this issue when I was telling him the exact same thing and noticed the Period one was inconsistent.

[philwebb]

I missed that conversation, is there more context for why the override was required?

[OrangeDog]

I was doing this

one had to support a slightly different format, registering theirs before ours.

and wanted a public constructor so I could delegate to an instance independently of a ConversionService.

And then I found DurationStyle.detectAndParse(String, ChronoUnit) and used that.

[snicoll]

Alright so case closed and let's make this one consistent. Thanks both.