-
Notifications
You must be signed in to change notification settings - Fork 40.5k
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
Fix typos in method names and javadoc #40971
Fix typos in method names and javadoc #40971
Conversation
…sure it is more understandable and coherent.
@@ -3,7 +3,7 @@ | |||
|
|||
The https://www.testcontainers.org/[Testcontainers] library provides a way to manage services running inside Docker containers. | |||
It integrates with JUnit, allowing you to write a test class that can start up a container before any of the tests run. | |||
Testcontainers is especially useful for writing integration tests that talk to a real backend service such as MySQL, MongoDB, Cassandra and others. | |||
Testcontainers are especially useful for writing integration tests that talk to a real backend service such as MySQL, MongoDB, Cassandra and others. |
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 is referring to the Testcontainers open source project. It should be singular.
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 have updated the code accordingly. Thank you for the feedback.
@@ -141,7 +141,7 @@ private void assertHasClassesOrLocations(MergedContextConfiguration mergedConfig | |||
boolean hasClasses = !ObjectUtils.isEmpty(mergedConfig.getClasses()); | |||
boolean hasLocations = !ObjectUtils.isEmpty(mergedConfig.getLocations()); | |||
Assert.state(hasClasses || hasLocations, | |||
() -> "No configuration classes or locations found in @SpringApplicationConfiguration. " | |||
() -> "No configuration classes or locations are found in @SpringApplicationConfiguration. " |
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 don't think "are" is needed 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.
Changes have been made, thank you for the feedback as always.
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.
Unnecessary Change: The original error message was already grammatically correct and clear. Adding "are" does not significantly enhance readability or understanding.
No Functional Impact: This change does not affect the functionality of the code in any way.
@@ -141,7 +141,7 @@ private void assertHasClassesOrLocations(MergedContextConfiguration mergedConfig | |||
boolean hasClasses = !ObjectUtils.isEmpty(mergedConfig.getClasses()); | |||
boolean hasLocations = !ObjectUtils.isEmpty(mergedConfig.getLocations()); | |||
Assert.state(hasClasses || hasLocations, | |||
() -> "No configuration classes or locations found in @SpringApplicationConfiguration. " | |||
() -> "No configuration classes or locations are found in @SpringApplicationConfiguration. " |
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.
Adding "are" doesn't significantly improve clarity
…sure it is more understandable and coherent. - resolved feedbacks
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 very much for the updates, @cmabdullah.
Thanks! |
Fix typos in method names and javadoc; update the documentation to ensure it is more understandable and coherent.