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

Revise readme headers and more edits #5713

Merged
merged 7 commits into from Jun 7, 2021
Merged

Revise readme headers and more edits #5713

merged 7 commits into from Jun 7, 2021

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Jun 4, 2021

Addressing part one of #5710.

@plotly/plotly_js

@archmoj archmoj mentioned this pull request Jun 4, 2021
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@archmoj archmoj requested a review from nicolaskruchten Jun 4, 2021
```sh
npm install plotly.js-dist-min
npm i --save plotly.js-dist-min
Copy link
Member

@nicolaskruchten nicolaskruchten Jun 7, 2021

Choose a reason for hiding this comment

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

don't we usually recommend plotly.js-dist instead of plotly.js-dist-min? In my (maybe out of date) experience, running a minifier on a minified bundle is really slow.

Copy link
Contributor Author

@archmoj archmoj Jun 7, 2021

Choose a reason for hiding this comment

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

Our minified bundle is not uglified, has smaller file size to download & load and no characters outside utf-8.
So there are benefits to use it IMO.

@nicolaskruchten
Copy link
Member

@nicolaskruchten nicolaskruchten commented Jun 7, 2021

LGTM 💃

@archmoj archmoj merged commit 551e8ed into master Jun 7, 2021
12 checks passed
@archmoj archmoj deleted the readme-headers branch Jun 7, 2021
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.

None yet

2 participants