Skip to content

Conversation

@code4cake
Copy link
Contributor

From this ISSUE thread

README.md Outdated
npm install javascripting
```

From within the `node_school` directory now run `node_modules/learnyounode/bin/javasripting` to start it.
Copy link
Member

Choose a reason for hiding this comment

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

Found some issues with this part, depending on the npm version, packages get installed in different routes.

How do you feel with something like this:


node_modules/learnyounode/bin/javascripting

(Or depending on the NPM version, it could be)

node_modules/.bin/javascripting

(PS: package name javascripting is misspelled)

Copy link
Contributor

Choose a reason for hiding this comment

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

npm bin refers to the path it is run. In a regular bash it is possible to simply run

 `npm bin`/learnyounode

(Also there is a typo for "javascripting" in the text)

@fforres
Copy link
Member

fforres commented Sep 7, 2016

Besides those 2 small details, LGTM 👍
(Also, kudos @dantesolis)

@code4cake
Copy link
Contributor Author

Hey guys fix the typos and sent another PR, let me know what you think. I added two pictures to the README, could a second pair of eyes double check the pics are displaying properly please.

@fforres
Copy link
Member

fforres commented Sep 11, 2016

LGTM 👍

@martinheidegger martinheidegger merged commit 6d0ca0a into nodeschool:source Sep 12, 2016
@martinheidegger
Copy link
Contributor

Thank you @dantesolis, this is a very nice addition to the Readme, thank you @fforres for the review!

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.

3 participants