Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

sigee
Copy link

@sigee 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")

Copy link
Member

@raboof raboof left a 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.

@garydgregory
Copy link
Member

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.

@sigee
Copy link
Author

sigee commented Jan 1, 2025

@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?

@garydgregory
Copy link
Member

Hello @sigee and happy new year
Yes, that's exactly what I mean.
I plan on doing such replacement whenever I see them.

@sigee
Copy link
Author

sigee commented Jan 1, 2025

I searched for them ("e.g.", "i.e.") in the main java files and replaced them. (I hope, not messed up too much)

@garydgregory
Copy link
Member

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.

@sigee
Copy link
Author

sigee commented Jan 1, 2025

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).
After that, you mentioned the replacements of the Latin abbreviation. So I went through the files and updated them accordingly. I extended the changes with 5 more files, but I think in this case the unified appearance is worth it to replace them at the same time. Unfortunately, in some cases, the line length changed significantly. This is what I meant by "not messed up too much".

@sigee
Copy link
Author

sigee commented Jan 2, 2025

@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.

@raboof raboof self-requested a review January 2, 2025 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants