Skip to content

Fix spelling#5349

Merged
archmoj merged 1 commit intoplotly:masterfrom
jbampton:fix-spelling
Dec 21, 2020
Merged

Fix spelling#5349
archmoj merged 1 commit intoplotly:masterfrom
jbampton:fix-spelling

Conversation

@jbampton
Copy link
Copy Markdown
Contributor

@jbampton jbampton commented Dec 17, 2020

Thanks for your interest in plotly.js!

Translations:

  • Please @ mention a few other speakers of this language who can help review your translations.
  • If you've omitted any keys from dist/translation_keys.txt - which means they will fall back on the US English text - just make a short comment about why in the PR description: the English text works fine in your language, or you would like someone else to help translating those, or whatever the reason.
  • You should only update files in lib/locales/, not those in dist/

Features, Bug fixes, and others:

Before opening a pull request, developer should:

  • git rebase their local branch off the latest master,
  • make sure to not git add the dist/ folder (the dist/ is updated only on version bumps),
  • make sure to commit changes to the package-lock.json file (if any new dependency required),
  • write an overview of what the PR attempts to do,
  • select the Allow edits from maintainers option (see this article for more details).

Note that it is forbidden to force push (i.e. git push -f) to remote branches associated with opened pull requests. Force pushes make it hard for maintainers to keep track of updates. Therefore, if required, please git merge master into your PR branch instead of git rebase master.

archmoj
archmoj reviewed Dec 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@archmoj archmoj left a comment

Choose a reason for hiding this comment

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

Looking good to me.
I am interested to know how you catch all these?
Please provide this info in the PR description instead of the template.
Thank you!

test/jasmine/tests/mock_test.js
figs['mapbox_density-mulitple'] = require('@mocks/mapbox_density-mulitple');
figs['mapbox_density-mulitple_legend'] = require('@mocks/mapbox_density-mulitple_legend');
figs['mapbox_density-multiple'] = require('@mocks/mapbox_density-multiple');
figs['mapbox_density-multiple_legend'] = require('@mocks/mapbox_density-multiple_legend');
Copy link
Copy Markdown
Contributor

@archmoj archmoj Dec 17, 2020

Choose a reason for hiding this comment

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

Could you please rename the mock and baseline files at following addresses?

test/image/mocks/mapbox_density-mulitple.json
test/image/mocks/mapbox_density-mulitple_legend.json
test/image/baselines/mapbox_density-mulitple_legend.png
test/image/baselines/mapbox_density-mulitple.png

test/jasmine/tests/range_slider_test.js
},
width: 800,
hieght: 500
height: 500
Copy link
Copy Markdown
Contributor

@archmoj archmoj Dec 17, 2020

Choose a reason for hiding this comment

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

Good catch.
Let's remove this line, so that the default height is applied as before.

@archmoj archmoj added community community contribution type: documentation labels Dec 17, 2020
@archmoj
Copy link
Copy Markdown
Contributor

archmoj commented Dec 21, 2020

💃
Merging...
I'll edit the requested changes in the next PR.

@archmoj archmoj merged commit a5b295f into plotly:master Dec 21, 2020
@jbampton jbampton deleted the fix-spelling branch December 21, 2020 13:51
@jbampton
Copy link
Copy Markdown
Contributor Author

jbampton commented Dec 21, 2020

Hey @archmoj I can do a PR now and fix them if you want ?

@archmoj
Copy link
Copy Markdown
Contributor

archmoj commented Dec 21, 2020

Hey @archmoj I can do a PR now and fix them if you want ?

I am about to submit a PR. So no worries there.
But please just edit the PR description here and if you let us know how you catch these? #5349 (review)
I am curious to know if we could possibly add a test to avoid these mistakes.

@archmoj archmoj mentioned this pull request Dec 21, 2020
Merged
jbampton added a commit to jbampton/plotly.js that referenced this pull request Dec 21, 2020
@jbampton
feat(CI): add a GitHub Action to check spelling
1dba7ca 
Found a better way to check spelling.  This Go tool does find mistakes in source code as well as Markdown. From my experience it does not work on all file types or find all the errors, but it does seem to work well on code comments
- refs plotly#5349
- refs plotly/dash#1081
Using -> https://github.com/client9/misspell
@jbampton jbampton mentioned this pull request Dec 21, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@archmoj archmoj archmoj left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

community community contribution

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jbampton @archmoj