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.

Sign up for GitHub

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

Jump to bottom

fixed multiple grammatical errors in docs. #1497

Merged
merged 5 commits into from
Aug 24, 2020
Merged

fixed multiple grammatical errors in docs. #1497

merged 5 commits into from
Aug 24, 2020

Conversation

vyasriday
Copy link
Contributor

Following files were updated

  1. README.md
  2. docs/error-handling.md
  3. docs/faq.md
  4. docs/guide.md

@vyasriday vyasriday changed the title fixed grammatical errors in docs. fixed multiple grammatical errors in docs. Aug 14, 2020
@codecov
Copy link

codecov bot commented Aug 14, 2020 •
edited

Codecov Report

Merging #1497 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1497   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          487       487           
  Branches       136       136           
=========================================
  Hits           487       487

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 68d97d6...acfc391. Read the comment docs.

@vyasriday vyasriday closed this Aug 14, 2020
@vyasriday vyasriday reopened this Aug 14, 2020
niftylettuce
niftylettuce reviewed Aug 22, 2020
View reviewed changes
docs/error-handling.md Outdated
@@ -52,7 +52,7 @@ app.use(async (ctx, next) => {

Error event listeners can be specified with `app.on('error')`.
If no error listener is specified, a default error listener
is used. Error listener receive all errors that make their
is used. Error listener receives all errors that make their
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be "Error listeners receive".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated with the changes

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you! Any chance you could pull in master branch and rebase? I added myself to list of people receiving vulnerability notifications as @dead-horse mentioned, because I would love to continue to help make sure Koa sticks around for the long term.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure I will rebase and then push the changes. @niftylettuce

niftylettuce
niftylettuce reviewed Aug 22, 2020
View reviewed changes
Readme.md Outdated Show resolved Hide resolved
miwnwski
miwnwski requested changes Aug 24, 2020
View reviewed changes
docs/guide.md Outdated
@@ -98,9 +100,9 @@ function logger(format) {
}
```

### Combining multiple middleware with koa-compose
### Combining multiple middlewares with koa-compose
Copy link
Member

Choose a reason for hiding this comment

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

Explained above.

docs/guide.md Outdated

Sometimes you want to "compose" multiple middleware into a single middleware for easy re-use or exporting. You can use [koa-compose](https://github.com/koajs/compose)
Sometimes you want to "compose" multiple middlewares into a single middleware for easy re-use or exporting. You can use [koa-compose](https://github.com/koajs/compose)
Copy link
Member

@miwnwski miwnwski Aug 24, 2020 •
edited

Choose a reason for hiding this comment

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

Explained below.

docs/faq.md Outdated
@@ -8,7 +8,7 @@
and less error-prone to write, for the entire stack, not
just the end application code.

Typically many middleware would
Typically many middlewares would
Copy link
Member

Choose a reason for hiding this comment

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

I disagree, the plural form of middleware is middleware.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the PR with the changes. I looked middleware and found a debatable answer. Both middleware and middlewares are used. But since middleware is uncountable as per english dictionary, let's use middleware for both singular and plural form moving forward. @niftylettuce what do you say?

docs/faq.md Outdated
@@ -23,7 +23,7 @@
## Does Koa include routing?

No - out of the box Koa has no form of routing, however
many routing middleware are available: https://github.com/koajs/koa/wiki
many routing middlewares are available: https://github.com/koajs/koa/wiki
Copy link
Member

Choose a reason for hiding this comment

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

Explained above.

docs/error-handling.md
@@ -52,7 +52,7 @@ app.use(async (ctx, next) => {

Error event listeners can be specified with `app.on('error')`.
If no error listener is specified, a default error listener
is used. Error listener receive all errors that make their
is used. Error listeners receive all errors that make their
Copy link
Member

Choose a reason for hiding this comment

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

I this case, it describes the function, which is an error listener "type". I would keep this in singular form IMO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With respect to line 53 I belive plural form makes more sense.

docs/error-handling.md
@@ -2,7 +2,7 @@

## Try-Catch

Using async functions means that you can try-catch `next`.
Using async functions mean that you can try-catch `next`.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not an english native speaker, but I feel it's supposed to be "means" rather than "mean" in this sentence. @niftylettuce What's your take on it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Either way works and people will understand

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@miwnwski I believe it should be mean rather than means as per grammar.

@niftylettuce niftylettuce merged commit 39e1a5a into koajs:master Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@niftylettuce niftylettuce niftylettuce left review comments

@dead-horse dead-horse dead-horse left review comments

@miwnwski miwnwski miwnwski requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@vyasriday @niftylettuce @dead-horse @miwnwski