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

doc: added a quick "Hello World" example in README #850

Closed
wants to merge 4 commits into from

Conversation

jimmyhsu
Copy link
Contributor

As per issue #786, added a quick and simple code sample to the build section for new users to test if they built from source correctly.

No source files were modified.

New code added to help completely new users test if their build-from-source is working correctly.
@jimmyhsu jimmyhsu changed the title Added a quick "Hello World" example to README.md readme: Added a quick "Hello World" example Feb 15, 2015
@jimmyhsu jimmyhsu changed the title readme: Added a quick "Hello World" example readme: added a quick "Hello World" example Feb 15, 2015
@@ -75,6 +75,11 @@ the binary verification command above.

## Build

To quickly test that io.js was built correctly from any of the steps below:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it makes sense to explain how to test the build before building. I'd move this to the end of the build instructions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wrestled with this myself, but I was afraid that leaving it at the bottom some people might miss it. I know personally I may have done so myself.

Copying it in each of the different OS instructions seems to be another option, but my thoughts are that it would make the readme too verbose.

Regardless, I'm with you on the logical progression as to where it should be placed. Bottom does make the most sense.

Thoughts or should we stick to putting it at the bottom?

Copy link
Member

Choose a reason for hiding this comment

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

I'm with @cjihrig. Why test something that isn't built yet? Regarding people not reading a readme -- well, that's not too much to do about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbergstroem, good point. I've updated the pull request with a new commit that moves it to the bottom.

@anonrig
Copy link
Member

anonrig commented Feb 16, 2015

Maybe it is better to use a function that wasn't implemented in the latest nodejs, but differs in iojs. Therefore, we may have a chance to show the new user the difference.

@jimmyhsu
Copy link
Contributor Author

That's a good idea, maybe place that in the resources for new users section?

My thoughts were to keep it short and simple just to test if the build was working.

@rvagg
Copy link
Member

rvagg commented Feb 16, 2015

iojs -e 'console.log(`Hello from iojs ${process.version}`)'

### Testing

To quickly test that io.js was built correctly from any of the steps above:
```
Copy link
Member

Choose a reason for hiding this comment

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

minor nit: add a space before the code block

also would be good to have the same code work on Windows and Unixes if you're in a position to check, if not then duplicate the block and for the Windows block use > instead of $

@jimmyhsu jimmyhsu changed the title readme: added a quick "Hello World" example doc: added a quick "Hello World" example in README Feb 16, 2015
@brendanashworth
Copy link
Contributor

To throw my coals onto the fire, I'd love to see a nodejs.org-like example with an HTTP server rather than a CLI-evaluated line. It doesn't have too much relevance to this PR specifically but it has more value to me than just "Hello".

@jimmyhsu
Copy link
Contributor Author

@brendanashworth I can probably take a tackle at that, but wouldn't that go in a place in the docs and not just the readme? If there's a place I can do that for sure, I'd be glad to do it.

On another note, I seem to be having trouble getting ES6 working in my terminal/command line on both OSX and Windows. Is this a known issue, or did I just set it up wrong?

@brendanashworth
Copy link
Contributor

@jimmyhsu I always liked how the nodejs.org example was easy to follow and kind of "in the spotlight", so I'd like to have it in the readme, but its more of a trivial thing.

Regarding ES6, I'd say open an issue, if it turns out its an error on your end or a known issue (that I don't know about) we can always close it.

@robertkowalski
Copy link
Contributor

@jimmyhsu looks good! tiny nit: can you rebase the PR against master, squash it to one commit and prefix it with the section you are editing (in this case doc:)? The first line of the description should be not longer than 50 chars (more details at https://github.com/iojs/io.js/blob/v1.x/CONTRIBUTING.md#step-3-commit)

thank you!

@bnoordhuis
Copy link
Member

can you rebase the PR against master

Did you mean v1.x? Master is months behind.

@robertkowalski
Copy link
Contributor

@bnoordhuis ah thanks yes, meant v1.x

@jimmyhsu
Copy link
Contributor Author

jimmyhsu commented Mar 3, 2015

@robertkowalski Sounds good! I'll tackle this later in the week and dig a bit more into why ES6 isn't working quite right.

@mscdex mscdex added the doc Issues and PRs related to the documentations. label Mar 22, 2015
@silverwind
Copy link
Contributor

ping @jimmyhsu

@Fishrock123
Copy link
Contributor

$ iojs -p process.version

We can use that for testing the build, but I think an actual example should be more thorough.

Qard pushed a commit that referenced this pull request Jun 25, 2015
PR-URL: #850
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
@Qard
Copy link
Member

Qard commented Jun 25, 2015

LGTM. Landed in 54d5437.

@Qard Qard closed this Jun 25, 2015
@rvagg rvagg mentioned this pull request Jun 30, 2015
mscdex pushed a commit to mscdex/io.js that referenced this pull request Jul 9, 2015
PR-URL: nodejs#850
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.