-
Notifications
You must be signed in to change notification settings - Fork 40.2k
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
Polish note about handling of caches that are created on-the-fly #19491
Conversation
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.
Thanks for the PR @izeye. I am not a native speaker but the proposed changes do not read as I intended them. I've added a comment.
@@ -1849,7 +1849,7 @@ The following cache libraries are supported: | |||
Metrics are tagged by the name of the cache and by the name of the `CacheManager` that is derived from the bean name. | |||
|
|||
NOTE: Only caches that are configured on startup are bound to the registry. | |||
For caches not defined in the cache’s configuration, that is caches created on-the-fly or programmatically after the startup phase, an explicit registration is required. | |||
For caches not defined in the cache’s configuration, that are created on-the-fly or programmatically after the startup phase, an explicit registration is required. |
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 really meant "that is" as "i.e.". Perhaps there's a missing comma somewhere?
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.
@snicoll I'm not an English native speaker, either 😅 I just thought that the "that" clause was meant to be for the "caches" as "that is caches" sounded unusual to me. I might be wrong here.
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.
OK. I am not sure either :) Would you be happy with
For caches not defined in the cache’s configuration, e.g. caches created on-the-fly or programmatically after the startup phase, ...
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.
@snicoll Sounds better to me 😄
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.
Sweet, would you be willing to update your proposal with that please?
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 PR polishes doc a bit.