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

doc: Fix typos and improve language in README.md #1669

Merged

Conversation

bernhardkaindl
Copy link
Contributor

@bernhardkaindl bernhardkaindl commented Apr 7, 2023

I think this is the first PR where the description turned out to be much longer than the diff in the "Files changed" view is. ;-)

  • This it's not scary at all, only a few characters here and there!
  • I just wanted to ensure you that it's not me/my preference, these are fixes for warnings generated by a tool.

These issues were found by using these LanguageTool extensions for VS Code:

Below are the warning messages from LanguageTool / LTeX extension.

:)


  • Missing commas after conjunctive/linking adverbs: "In addition," and "Also,".
  • Use a comma before 'so' if it connects two independent clauses unless they are closely connected and short
    • (Which but aren't short, so the fix is applied)
  • Use "this info" instead of "it" for clarity
  • "inside of": This phrase is redundant. Consider using 'inside'
  • The verb 'set up' is spelled as two words. The noun 'setup' is spelled as one.
  • Multi-process and multithreaded: These are normally spelled as one.

Also, I reworded a sentence which included the sequence "the write" which it warned about with this explanation:
"After 'The', the verb 'write' doesn't fit. If 'write' is the first word in a compound adjective, use a hyphen between the two words. Using the verb 'write' as a noun may be non-standard."

Also, it found a typo and words which need to be capitalized, like Chrome (The proper noun 'Chrome' (= software from Google) needs to be capitalized) which I fixed as well.


:)

Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, sometimes we need long description even for an oneliner. :)

README.md Outdated
In addition, function arguments and return values can be saved and shown later.

It supports multi-process and/or multi-threaded applications. With root
It supports multiprocess and/or multithreaded applications. With root
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I still prefer having a dash.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good, change is reverted now.

@bernhardkaindl bernhardkaindl force-pushed the README.md-fixes-by-LanguageTool branch from 03235cf to 788b30a Compare April 12, 2023 12:49
@bernhardkaindl bernhardkaindl requested a review from namhyung April 12, 2023 12:50
@bernhardkaindl bernhardkaindl changed the title README.md: Fix many minor spelling issues found by LanguageTool README.md: Fix spelling issues found by LanguageTool Apr 12, 2023
@bernhardkaindl bernhardkaindl changed the title README.md: Fix spelling issues found by LanguageTool README.md: Apply spelling fixes suggested by LanguageTool Apr 12, 2023
Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late review. Now it looks good. But can you please add your sign-off in the commit message?

@namhyung
Copy link
Owner

Also, I think the commit subject can have the 'doc:' prefix instead of 'README.md:'.

@namhyung namhyung added the doc label Jun 9, 2023
@bernhardkaindl bernhardkaindl force-pushed the README.md-fixes-by-LanguageTool branch 2 times, most recently from 7a179da to 518ee16 Compare November 29, 2023 06:25
@bernhardkaindl bernhardkaindl changed the title README.md: Apply spelling fixes suggested by LanguageTool doc: Fix typos and improve language in README.md Nov 29, 2023
@bernhardkaindl bernhardkaindl force-pushed the README.md-fixes-by-LanguageTool branch from 518ee16 to 9738568 Compare November 29, 2023 06:28
@bernhardkaindl
Copy link
Contributor Author

@namhyung Thanks for the approval, and I updated the commit message as requested. You can merge now.

@namhyung
Copy link
Owner

It'd be nice to have your real email address in the sign-off line.

@honggyukim
Copy link
Collaborator

Hi @bernhardkaindl, good to see you back here again.

It'd be nice to have your real email address in the sign-off line.

@namhyung mentioned the line below.

Signed-off-by: Bernhard Kaindl <43588962+bernhardkaindl@users.noreply.github.com>

I don't think 43588962+bernhardkaindl@users.noreply.github.com is your real email address so please fix it.

- Missing commas after conjunctive/linking adverbs: addition,also.
- Use a comma before 'so' if it connects two independent clauses
  unless they are closely connected and short (but aren't short)
- Use "this info" instead of "it" for clarity
- "inside of": This phrase is redundant. Consider using 'inside'
- The verb 'set up' is spelled as two words. The noun 'setup' is
  spelled as one.

Signed-off-by: Bernhard Kaindl <bernhardkaindl7@gmail.com>
@bernhardkaindl bernhardkaindl force-pushed the README.md-fixes-by-LanguageTool branch from 9738568 to b806877 Compare December 1, 2023 16:28
@bernhardkaindl
Copy link
Contributor Author

bernhardkaindl commented Dec 1, 2023

I don't think 43588962+bernhardkaindl@users.noreply.github.com is your real email address so please fix it.

@honggyukim @namhyung
This is just the standard mail address you make commits on GitHub when your email address is set to private, and as DCO is accepting it, I would not have considered as a problem, it's standard, even in the uftrace commits:

git log|grep -c noreply.github.com
20

However, as I don't want to argue on that a lot, I updated the commit.

@namhyung
Copy link
Owner

namhyung commented Dec 2, 2023

Yeah, it's from old-style email conversations in the kernel community where I used to work on. Probably not needed for github based workflow, but I still prefer seeing real email addresses. Thanks for the update!

Copy link
Owner

@namhyung namhyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@namhyung namhyung merged commit 00422e2 into namhyung:master Dec 2, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants