-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
improve integration doc #9322
base: main
Are you sure you want to change the base?
improve integration doc #9322
Conversation
Thanks for your pull request! This pull request does not follow the contribution rules. Could you have a look? ❌ All commit messages should start with a JIRA issue key matching pattern › This message was automatically generated. |
@@ -35,17 +35,15 @@ What's interesting is the `ServiceRegistry` and the pluggable swapping of the di | |||
The basic requirement for a `Service` is to implement the marker interface `org.hibernate.service.Service`. | |||
Hibernate uses this internally for some basic type safety. | |||
|
|||
The `Service` can also implement a number of optional life-cycle related contracts: | |||
The `Service` can also implement a number of optional lifecycle related contracts: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change this to maintain consistency. lifecycle
is used elsewhere
|
||
`org.hibernate.service.spi.Startable`:: | ||
allows the `Service` impl to be notified that it is being started and about to be put into use. | ||
`org.hibernate.service.spi.Stoppable`:: | ||
allows the `Service` impl to be notified that it is being stopped and will be removed from use. | ||
`org.hibernate.service.spi.ServiceRegistryAwareService`:: | ||
allows the `Service` to be injected with a reference to the registry that is managing it. See <<services-dependencies>> for more details. | ||
`org.hibernate.service.spi.Manageable`:: | ||
marks the `Service` as manageable in JMX provided the JMX integration is enabled. This feature is still incomplete. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The above stuff has been absent from the codebase
@@ -118,12 +116,12 @@ Each specific type of registry defines its own `ServiceInitiator` specialization | |||
=== Types of ServiceRegistries | |||
|
|||
Currently Hibernate utilizes three different `ServiceRegistry` implementations forming a hierarchy. | |||
Each type is a specialization for the purpose of type-safety, but they add no new functionality. | |||
Each type is a specialization for the purpose of type safety, but they add no new functionality. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for consistency reason. type safety
is used more often elsewhere.
@@ -1,4 +1,4 @@ | |||
public interface EventPublishingService extends Service { | |||
|
|||
public void publish(Event theEvent); | |||
void publish(Event theEvent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we don't need to specify public
for interface method
.get( JMS_CONNECTION_FACTORY_NAME_SETTING ); | ||
this.destinationName = configurationValues | ||
this.destinationName = (String) configurationValues | ||
.get( JMS_DESTINATION_NAME_SETTING ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added generics usage as much as possible in this PR for using Map
alone seems sloppy
@@ -1,4 +1,4 @@ | |||
public class LatestAndGreatestConnectionProviderImplContributor1 | |||
public class LatestAndGreatestConnectionProviderImplContributor | |||
implements ServiceContributor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally the example code snippets were prepared for two flavours:
- LatestAndGreatestConnectionProviderImplContributor1
- LatestAndGreatestConnectionProviderImplContributor2
one of the reasons is two meta-inf
files were provided so using number suffix to differentiate between them:
- ex2-meta-inf
- ex3-meta-inf
However, the ex3-meta-inf
was not used at all so it is deleted in this PR. Then it seems no such confusing suffix is needed.
selector.registerStrategyImplementor( | ||
ConnectionProvider.class, | ||
"lag", | ||
LatestAndGreatestConnectionProviderImpl.class | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this code snippet is too sloppy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should be requireService()
instead of getService()
if we're being picky.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, we should use that, otherwise NPE later can't be avoided.
documentation/src/main/asciidoc/integrationguide/chapters/services/Services.adoc
Outdated
Show resolved
Hide resolved
documentation/src/main/asciidoc/integrationguide/chapters/services/Services.adoc
Outdated
Show resolved
Hide resolved
documentation/src/main/asciidoc/integrationguide/chapters/services/Services.adoc
Outdated
Show resolved
Hide resolved
|
||
// if we wanted to allow other strategies (e.g. a jms | ||
// queue publisher) we might also register short names | ||
// queue publisher) we might also register short-names |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hyphenating "short names" is just weird. You don't need a hyphen between an adjective and a noun.
A hyphen would be used if this were a compound adjective, but it's not. So if there are other occurrences of "short-name", they should be fixed by removing the hyphen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed other 'short-name's to 'short name's as you suggested, in addition to this one.
…ices/Services.adoc Co-authored-by: Gavin King <[email protected]>
…ices/Services.adoc Co-authored-by: Gavin King <[email protected]>
…ices/Services.adoc Co-authored-by: Gavin King <[email protected]>
Recently working on Hibernate Integration stuff in MongoDB. After carefully reading the excellent Integration doc, it seems there are some minor defects that could be fixed.
As normal, many might be false positives. I'll leave some comments to explain the reasons behind some changes.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.