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

Added information on ES5 vs ES6+ #82

Merged
merged 1 commit into from Dec 25, 2020
Merged

Added information on ES5 vs ES6+ #82

merged 1 commit into from Dec 25, 2020

Conversation

thkruz
Copy link
Collaborator

@thkruz thkruz commented Dec 23, 2020

As promised, took a stab at doing a pull request properly.

Just included a short summary of the discussion on ES5 vs ES6+ and why satellite.min.js isn't shown on the github page.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 82.662% when pulling ea28d25 on thkruz:update-readme into 4626ff5 on shashwatak:develop.

@davidcalhoun
Copy link
Contributor

davidcalhoun commented Dec 23, 2020

Thanks for adding this clarification! I think some of the confusion is from folks who are no so familiar with npm. There is actually a missing step as well: running npm run build, which will output the /dist directory.

I checked out package.json and just saw a build that runs with prepublishOnly. An alternate solution: if this is changed to prepare, it will also run on npm install: https://docs.npmjs.com/cli/v6/using-npm/scripts, so folks wouldn't need to manually run npm run build.

EDIT: My bad, the readme has a Building section that explains how to build!

@ezze ezze merged commit c726d15 into shashwatak:develop Dec 25, 2020
@ezze
Copy link
Collaborator

ezze commented Dec 25, 2020

@thkruz Thanks a lot, mate!

@davidcalhoun Nice catch! We should consider to change prepublishOnly to prepare for those who are eager for old school approach without npm.

@thkruz
Copy link
Collaborator Author

thkruz commented Dec 25, 2020

Thanks for adding this clarification! I think some of the confusion is from folks who are no so familiar with npm.

@davidcalhoun 100% was my issue. Spent the last few days learning/implementing it after @ezze's explanation and now it all makes perfect sense.

@thkruz thkruz deleted the update-readme branch December 31, 2020 19:33
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.

None yet

4 participants