-
Notifications
You must be signed in to change notification settings - Fork 41.2k
Polish #43879
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 #43879
Conversation
0af359c
to
1ccf7de
Compare
@@ -402,7 +402,7 @@ You can also add javadoc:org.springframework.test.context.DynamicPropertySource[ | |||
|
|||
When using devtools, you can annotate beans and bean methods with javadoc:org.springframework.boot.devtools.restart.RestartScope[format=annotation]. | |||
Such beans won't be recreated when the devtools restart the application. | |||
This is especially useful for Testcontainer javadoc:org.testcontainers.containers.Container[] beans, as they keep their state despite the application restart. | |||
This is especially useful for Testcontainers javadoc:org.testcontainers.containers.Container[] beans, as they keep their state despite the application restart. |
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 think this was intentionally singular. If it's to change, I think it should be changed to "container".
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.
@wilkinsona How about "Testcontainers'" or just removing "Testcontainer"?
"container Container beans" doesn't sound right 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.
Oops. Yeah, I think we can just remove it as we're already in a section about Testcontainers. I think "for Container beans" reads better and there's enough context to know that it's a Testcontainers container.
@@ -1926,7 +1926,7 @@ | |||
}, | |||
{ | |||
"name": "management.metrics.graphql.autotime.enabled", | |||
"description": "Whether to automatically time web client requests.", | |||
"description": "Whether to automatically time GraphQL requests.", |
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've opened #43901 for this as I think it warrants a separate issue that'll appear in the changelog.
@@ -35,7 +35,7 @@ class FilteringStatusListener extends ContextAwareBase implements StatusListener | |||
|
|||
/** | |||
* Creates a new {@link FilteringStatusListener}. | |||
* @param delegate the {@link StatusListener} delegate to | |||
* @param delegate the {@link StatusListener} to delegate |
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 think this should be "to delegate to".
Signed-off-by: Johnny Lim <[email protected]>
Signed-off-by: Johnny Lim <[email protected]>
b359ef3
to
cbb44e1
Compare
@wilkinsona Thanks for the feedback! I applied your suggestions in cbb44e1. |
…nabled Signed-off-by: Johnny Lim <[email protected]>
a4a3861
to
cadf0e0
Compare
Sorry, I forgot to revert the change on the description of the |
See gh-43879 Signed-off-by: Johnny Lim <[email protected]>
Thanks very much, @izeye! |
See spring-projectsgh-43879 Signed-off-by: Johnny Lim <[email protected]> Signed-off-by: arefbehboudi <[email protected]>
This PR polishes a bit.