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

Documentation mitigation for #176 #177

Merged
merged 2 commits into from Jul 21, 2017
Merged

Documentation mitigation for #176 #177

merged 2 commits into from Jul 21, 2017

Conversation

nheminger
Copy link
Contributor

No description provided.

Puts in an extra step in the README to address the dependency that
rollup js be installed globally for now, so that it can be invoked via
the shell/command.
@jgravois
Copy link
Contributor

jgravois commented Apr 18, 2017

rollup doesn't need to be installed globally when you invoke the grunt command using npm run build. it'd be better to document that.

@nheminger
Copy link
Contributor Author

Good catch. I'm not as familiar with that. Is npm run build a way to invoke grunt or it is invoking npm? Would it be recommended for users to run npm run build over grunt?

@jgravois
Copy link
Contributor

Would it be recommended for users to run npm run build over grunt?

exactly. npm scripts are defined in package.json and in this case provide nothing more than a little encapsulation.

more info here:
https://blog.jayway.com/2014/03/28/running-scripts-with-npm/

@rwmajor2 rwmajor2 merged commit 92b9931 into Esri:master Jul 21, 2017
@jgravois
Copy link
Contributor

jgravois commented Jul 21, 2017

@rwmajor2 i had requested changes on this pull request. any reason in particular you wanted to include instructions for installing rollup globally in the doc?

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

3 participants