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

style(mixed): fix spelling and grammar issues in comments and tests #2196

Merged
merged 2 commits into from
Oct 16, 2017

Conversation

direvus
Copy link
Contributor

@direvus direvus commented Oct 14, 2017

This PR corrects some spelling and grammar errors in docs, code comments and example content.

Mostly its/it's errors in this commit, along with a few verb number
errors.
@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2196   +/-   ##
=======================================
  Coverage   99.73%   99.73%           
=======================================
  Files         151      151           
  Lines        2611     2611           
=======================================
  Hits         2604     2604           
  Misses          7        7
Impacted Files Coverage Δ
src/addons/Ref/Ref.js 100% <ø> (ø) ⬆️
src/modules/Accordion/Accordion.js 100% <ø> (ø) ⬆️
src/elements/Input/Input.js 100% <ø> (ø) ⬆️
src/elements/Divider/Divider.js 100% <ø> (ø) ⬆️

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 70985ae...1e9a48b. Read the comment docs.

@layershifter layershifter changed the title Spelling/grammar fixes style(mixed): fix spelling and grammar issues in comments and tests Oct 14, 2017
Copy link
Member

@layershifter layershifter left a comment

Choose a reason for hiding this comment

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

@direvus Thanks for your eyes 👍

@@ -29,7 +29,7 @@ export default class Ref extends Component {
componentDidMount() {
const { innerRef } = this.props

// Heads up! Don't move this condition, it's a short circle that avoids run of `findDOMNode`
// Heads up! Don't move this condition, it's a short circuit that avoids run of `findDOMNode`
Copy link
Member

Choose a reason for hiding this comment

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

This one made me chuckle :)

@levithomason levithomason merged commit 2d12b79 into Semantic-Org:master Oct 16, 2017
@levithomason
Copy link
Member

Very cool, thanks much. There are quite a few of these in the code base and I'd love to clean them all up.... priorities!

@direvus
Copy link
Contributor Author

direvus commented Oct 16, 2017

@levithomason You're very welcome! I spotted a couple of typos in the documentation and then went on a bit of a rampage. I've checked every instance of it's/its in the codebase, so I'm pretty confident that those are all correct (for now anyway).

There were a fair number of code comments I came across that seemed non-idiomatic or strangely worded, but I was reluctant to fix those without a solid understanding of what each comment intends to say.

If there are any other particular spelling/grammar errors you think might be widespread in the repo, feel free to let me know and I'll do a sweep.

@levithomason
Copy link
Member

levithomason commented Oct 16, 2017

Awesome, I think the props doc blocks could use a sweep. As well as the index.js files in docs/app/Examples. These contain <ComponentExample />s which have title and description props. That is what powers the text above every example in the docs.

Many of our contributors are non-native English speakers and awesome developers so I've consciously chosen not to slow them down for grammar's sake. However, a clean sweep over any of this would be super appreciated!

@levithomason
Copy link
Member

Released in semantic-ui-react@0.75.0

@direvus direvus deleted the spelling-grammar branch October 16, 2017 08:51
@direvus
Copy link
Contributor Author

direvus commented Oct 16, 2017

@levithomason No worries, I'll have a look soon.

With the props doc blocks, does it make sense to change both the *.js and the *.d.ts files, or is one of these generated from the other?

@levithomason
Copy link
Member

Unfortunately, we manage both manually. Please update both to match.

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

Successfully merging this pull request may close these issues.

4 participants