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

docs: fixed typos, spaces, and punctuation #1482

Merged
merged 7 commits into from
Apr 11, 2024

Conversation

grjan7
Copy link
Contributor

@grjan7 grjan7 commented Apr 10, 2024

Fixes typos, spaces and punctuation.

@crandmck
Copy link
Member

Thanks for the PR @grjan7

Based on what I saw so far, this all looks OK, but I stopped at the edits to the _includes/readmes. These files live in the corresponding repo and are simply copied here using get-readmes.sh.
So, e.g., https://expressjs.com/en/resources/middleware/body-parser.html comes from https://github.com/expressjs/body-parser/blob/master/README.md.

So the files should be edited in each individual repo, not here. There is a notice at the top of each such page on the site:

Note: This page was generated from the xyz README.

We really should explain this somewhere, and it would be way better if it were done in a GitHub workflow instead of manually running a script.

IAC, please remove these files from the PR, and then I'll continue reviewing.
Also, you should know that we plan to remove some pages that are linked from the "Resources" and "Advanced" menus. cf. https://github.com/expressjs/expressjs.com/wiki/Discuss:-Remove-pages-from-docs.
You might want to remove these files from the PR, too. If we land that PR (which doesn't yet exist, but soon will) before you re-submit this one, you'll need to remove them from the PR.

@crandmck crandmck self-requested a review April 10, 2024 23:08
Copy link
Member

@crandmck crandmck left a comment

Choose a reason for hiding this comment

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

Remove files in _includes/readmes per comment.

@grjan7
Copy link
Contributor Author

grjan7 commented Apr 11, 2024

Ok, @crandmck.

@grjan7
Copy link
Contributor Author

grjan7 commented Apr 11, 2024

@crandmck, I have removed those files from PR. You can review them now.

en/starter/installing.md Outdated Show resolved Hide resolved
@@ -120,9 +120,9 @@ app.listen(3000, () => {

[http-terminator](https://github.com/gajus/http-terminator) implements logic for gracefully terminating an express.js server.
Copy link
Member

Choose a reason for hiding this comment

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

terminating an Express server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

en/guide/overriding-express-api.md Outdated Show resolved Hide resolved
@crandmck crandmck self-requested a review April 11, 2024 17:52
@crandmck crandmck merged commit 8bc34dd into expressjs:gh-pages Apr 11, 2024
4 checks passed
@crandmck
Copy link
Member

Thanks again @grjan7.

Corrections to spelling, punctuation, and formatting are valuable.

Many of the edits in this PR simply removed extra spaces. I don't feel that adds a lot of value. It doesn't do any harm either, so I left them in this PR, but just for future reference.

@grjan7
Copy link
Contributor Author

grjan7 commented Apr 12, 2024

Thanks for your input, @crandmck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants