-
Notifications
You must be signed in to change notification settings - Fork 160
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 grammar issues in the JavaDoc comments #308
base: master
Are you sure you want to change the base?
Conversation
sigee
commented
Dec 29, 2024
- Comparison requires 'than', not 'then'.
- Use 'a' instead of 'an' or vice versa.
- The abbreviations 'i.e.' and 'e.g.' requires two periods.
- Some words are spelled with hyphen
- Some words spelled as one word instead of two
- Repeated words (e.g. 'the the')
- Missing comma
- Plural instead of possessive
- Possessive instead of "it's" (short for "it is")
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.
while I don't find all of these changes improvements (in particular I don't think those comma's are necessary/helpful), some are clear fixes, so this seems like a net improvement +1.
I would replace "i.e." and "e.g." and other Latin abbreviation with their English equivalent. Back when I was writing books, our style guide did allow their use. Most non-native speakers (and some native) don't know the difference to boot. |
@garydgregory I am not a native English speaker, and I am unsure what you mean by replacing them with their English equivalents. I understand the meanings of 'i.e.' and 'e.g.' in English, but I am not certain whether you are referring to their full expressions, such as 'that is' for 'i.e.' or 'for example' for 'e.g.' Wouldn't that be somewhat excessive? |
Hello @sigee and happy new year |
I searched for them ("e.g.", "i.e.") in the main java files and replaced them. (I hope, not messed up too much) |
Oh please don't make me review 32 files. The original PR was a single file? Or two? When I do this, I usually read the Javadoc to make sure it actually sounds good, as opposed to search, replace, and hope for the best. I have no idea what you did of course. |
I am sorry for the extra work, but the original change contained 27 files already. So just 5 more files changed. The original changes were typical grammar mistakes reported by my IDE (IntelliJ). |
src/main/java/org/apache/commons/validator/routines/CreditCardValidator.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/commons/validator/routines/CodeValidator.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/commons/validator/routines/UrlValidator.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/commons/validator/routines/UrlValidator.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/commons/validator/routines/package-info.java
Outdated
Show resolved
Hide resolved
@sebbASF Thank you for your review. I did the changes you requested. Only the last one is left, where the original text was "i.e." and I changed it to "that is", but you said "Does not make sense to me as either "that is" or "for example.". For me, "taht is" seems ok, because there are all of the relevant validators listed as explanation. |