Skip to content

Conversation

@Sajjon
Copy link
Contributor

@Sajjon Sajjon commented Nov 8, 2019

Formatting
Grammar & punctuation
Links

This is the same PR as closed #33 (but in all my excitement I accidentally committed from my master, branch, this PR is from a feature branch). I've rebased merged #34 onto my branch before applying the changes.

All changes suggested by @xwu (in #33) has been applied here, except an incorrect comma.

In general, I would really like to recommend using amazing grammar and language precision tool Grammarly, which I used to find some room for improvement.

README.md Outdated
In practice, it will act as a staging ground for some APIs that may eventually be incorporated into the Swift Standard Library, and when that happens such changes will be proposed to the Swift Standard Library using the established evolution process of the Swift project.

Swift Numerics uses github issues to track bugs and features, and pull requests for development.
Swift Numerics uses GitHub issues to track bugs and features and pull requests for development.
Copy link
Contributor

@xwu xwu Nov 8, 2019

Choose a reason for hiding this comment

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

Suggested change
Swift Numerics uses GitHub issues to track bugs and features and pull requests for development.
Swift Numerics uses GitHub issues to track bugs and features, and pull requests for development.

This comma delimiter is required for clarity.

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But ", and" is grammatically incorrect... Do you like this, it is both grammatically correct and easy to read:

Alternative 1 - Ampersand

"Swift Numerics uses GitHub issues to track bugs & features and pull requests for development."

Alternative 2 - Slash:

"Swift Numerics uses GitHub issues to track bugs/features and pull requests for development."

Alternative 3 - "or":

"Swift Numerics uses GitHub issues to track bugs or features and pull requests for development."

🤔

Copy link
Member

Choose a reason for hiding this comment

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

Those are all bad.

Swift Numerics uses GitHub issues to track bugs and features. We use pull requests for development.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stephentyrone great, fixed! Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

But ", and" is grammatically incorrect...

FWIW, it is grammatically just fine here, whatever the tools try to tell you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xwu It is the case of compound predicate, see screenshot:

Screenshot 2019-11-08 at 17 05 25

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no second verb. Even in the case of a compound predicate, it's fine to use a comma when it's necessary for other reasons. You can ignore the tool.

@stephentyrone
Copy link
Member

Except for xiaodi's nit, this LGTM. Please take that fix and I'll merge.

Formatting
Grammar & punctuation
Links
Copy link
Member

@stephentyrone stephentyrone left a comment

Choose a reason for hiding this comment

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

Thanks!

@stephentyrone stephentyrone merged commit 4a23df7 into apple:master Nov 8, 2019
@Sajjon Sajjon deleted the update_readme_links_grammar branch November 8, 2019 15:57
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.

3 participants