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-484: API/SPI/internal split #105

Closed
wants to merge 16 commits into from

Conversation

gunnarmorling
Copy link
Member

The pull request comprises the following changes:

  • Moved everything from the non public packages to org.hibernate.validator.internal
  • Removed references from public packages to internal packages where possible

There are still some references from public packages to internal ones which we can't remove now without breaking existing client code based on the public API. As discussed with Hardy, we'll address these dependencies in HV 5.0 where some breaking changes will be inevitable anyways.

I created a Clirr report for the change and didn't notice any changes to the public API relevant to clients.

@emmanuelbernard
Copy link
Member

Hibernate Search uses the following rules:

  • use .impl (not internal)
  • .spi and .impl must be last in the package name. You can't have org.hibernate.validator.impl.foo

Do you want and follow the same rules?

@gunnarmorling
Copy link
Member Author

  • use .impl (not internal)

Personally I'm preferring internal over impl as it better communicates the intention IMO, that is the package is internal and not intended for client use, no matter whether the contained types actually are implementations or interfaces etc.
That said I think it makes sense to align this over the Hibernate projects. In Core I saw internal packages, while in Search impl is used. Do you know which one is recommended? We should take that one for HV, too.

  • .spi and .impl must be last in the package name. You can't have org.hibernate.validator.impl.foo

After discussing the options, Hardy and I came to the conclusion that we prefer the approach with api, spi and impl/internal up in the package hierarchy. That way one has clearly separated public and private parts of the code base, instead of disseminating the API/SPI/internal parts over several sub-packages.
This also opens the possibility to split the code base into two modules/JARs, one to be used as compile dependency (API/SPI), one as runtime dependency (impl). We haven't planned this for now, but that way we could do so later on. Another advantage is that this approach avoids that technically motivated packages (api, spi, impl) live on one level next to packages motived by the "domain" (constraints, engine etc.), and this again and again for every sub-package.
Finally I think distinguishing public and internal parts on the top level should make some practical things simpler like creating API change reports, publishing API JavaDoc etc..

@emmanuelbernard
Copy link
Member

I saw internal packages, while in Search impl is used. Do you know which one is recommended? We should take that one for HV, too.

We use impl in search to be shorter.

After discussing the options, Hardy and I came to the conclusion that we prefer the approach with api, spi and impl/internal up in the package hierarchy.

OK. For the record, I used almost the same arguments to get the technical level at the bottom ;)
Be aware that your approach is not the rule followed by Hibernate ORM and for sure not the rule followed by Hibernate Search and Hibernate OGM.

@gunnarmorling
Copy link
Member Author

I've renamed internal to impl now to follow the style from Search.

I used almost the same arguments to get the technical level at the bottom ;)

Hmmm, that's interesting. I hoped these arguments would be absolutely convincing to do it the way I/we did it ;-) I've also checked some other projects (e.g. JDT, Aether etc.) and it seemed to me that the "distinction-at-top-level" approach is more common.

@hferentschik
Copy link
Contributor

Looking...

@hferentschik
Copy link
Contributor

Personally I'm preferring internal over impl as it better communicates the intention IMO, that is the package is internal and > not intended for client use, no matter whether the contained types actually are implementations or interfaces etc.
That said I think it makes sense to align this over the Hibernate projects. In Core I saw internal packages, while in Search > impl is used. Do you know which one is recommended? We should take that one for HV, too.

Same here, I think internal is better. impl is biased, because it is in most cases used to distinguish between an interface and its implementation. Core uses also internal so it is hard to align consistently across all projects.

@hferentschik
Copy link
Contributor

That way one has clearly separated public and private parts of the code base, instead of disseminating the API/SPI
/internal parts over several sub-packages.
This also opens the possibility to split the code base into two modules/JARs, one to be used as compile dependency
(API/SPI), one as runtime dependency (impl). We haven't planned this for now, but that way we could do so later on.
Another advantage is that this approach avoids that technically motivated packages (api, spi, impl) live on one level next > to packages motived by the "domain" (constraints, engine etc.), and this again and again for every sub-package.
Finally I think distinguishing public and internal parts on the top level should make some practical things simpler like
creating API change reports, publishing API JavaDoc etc..

+1 to all of these arguments

@hferentschik
Copy link
Contributor

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants