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 English issues in the documentation #4649

Merged
merged 24 commits into from
Nov 16, 2024

Conversation

rico-chet
Copy link
Contributor

@rico-chet rico-chet commented Nov 15, 2024

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt and RELEASE.txt (and read the README.rst).
  • I have updated the appropriate documentation

The change is probably too minor to check any of the boxes above.

Addresses #4635

@rico-chet
Copy link
Contributor Author

The first commit contains the obvious things which are likely to be OK without a debate. It can be cherry-picked and merged separately to avoid merge conflicts with other branches.

@@ -1033,7 +1033,7 @@ either as separate arguments to the
&f-Clean;
method, or as a list.
&f-Clean;
will also accept the return value of any of the &consenv;
will also accept the return value of the &consenv;
Copy link
Collaborator

Choose a reason for hiding this comment

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

A similar change is also in #4648

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feel free to merge #4648 first, I'm OK to resolve the conflicts.

@@ -1065,7 +1065,7 @@ Clean(['foo', 'bar'], 'something_else_to_clean')
In this example,
installing the project creates a subdirectory for the documentation.
This statement causes the subdirectory to be removed
if the project is deinstalled.
if the project is uninstalled.
Copy link
Collaborator

Choose a reason for hiding this comment

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

A similar change is also in #4648

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feel free to merge #4648 first, I'm OK to resolve the conflicts.

@@ -350,15 +350,15 @@ python -m pip install ninja
<summary>
<para>
Internal value used to specify the function to call with argument env to generate the list of files
which if changed would require the &ninja; build file to be regenerated.
which -- if changed -- would require the &ninja; build file to be regenerated.
Copy link
Collaborator

Choose a reason for hiding this comment

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

not super fond of double dashes as separators, though that may be only a personal bias.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can also use parentheses instead if that gets this PR going.

Copy link
Contributor

Choose a reason for hiding this comment

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

proper grammar would be which, if changed, would...

@@ -40,7 +40,7 @@
This paper has introduced &SCons;, a next-generation build tool
with a modular, embeddable architecture and a direct Python
interface. &SCons; has a global view of the dependencies in a source
tree, uses MD5 signatures to decide if derived files are out of date,
Copy link
Collaborator

Choose a reason for hiding this comment

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

this file is "unmaintained" - doesn't hurt to fix this, but it has worse problems.

@mwichmann
Copy link
Collaborator

In general, this looks good to me, not just fixing typos but improving consistency. The one question would be whether our esteemed maintainer indeed prefers normalizing on "out-of-date" instead of "out of date" (I'm fine with that change).

@bdbaddog
Copy link
Contributor

@rico-chet - you should also add a blurb in RELEASE.txt and CHANGES.txt per the checklist at the top.

@bdbaddog bdbaddog linked an issue Nov 16, 2024 that may be closed by this pull request
@bdbaddog
Copy link
Contributor

Made a bunch of edits, added CHANGE/RELEASE.txts I'll go ahead an merge.

If you can let me know your actual name we can update the CHANGES.txt blurb for this PR.
Thanks for the great work!

@bdbaddog bdbaddog merged commit 630ee1c into SCons:master Nov 16, 2024
@mwichmann mwichmann added this to the NextRelease milestone Nov 17, 2024
@rico-chet
Copy link
Contributor Author

If you can let me know your actual name we can update the CHANGES.txt blurb for this PR. Thanks for the great work!

My actual name is Alex Thiessen, and it did cost me a few hours of spare time to bring it in. I use SCons at work, find it underappreciated, and am glad to give something back.

@bdbaddog
Copy link
Contributor

@rico-chet - updated CHANGES.txt - Thanks for your help!

@rico-chet rico-chet deleted the fix-languagetool-complaints branch December 7, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

Documentation is littered with typos
3 participants