FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: added a quick "Hello World" example in README by jimmyhsu · Pull Request #850 · nodejs/node · GitHub

/ node Public

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

Closed
jimmyhsu wants to merge 4 commits into
nodejs:v1.xfrom
jimmyhsu:patch-2
Closed

doc: added a quick "Hello World" example in README#850
jimmyhsu wants to merge 4 commits into
nodejs:v1.xfrom
jimmyhsu:patch-2

Conversation

Copy link
Copy Markdown
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 changed the title Added a quick "Hello World" example to README.md readme: Added a quick "Hello World" example Feb 15, 2015
jimmyhsu changed the title readme: Added a quick "Hello World" example readme: added a quick "Hello World" example Feb 15, 2015
Comment thread README.md Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

anonrig commented Feb 16, 2015

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
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 commented Feb 16, 2015

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

Comment thread README.md Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 changed the title readme: added a quick "Hello World" example doc: added a quick "Hello World" example in README Feb 16, 2015

Copy link
Copy Markdown
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".

Copy link
Copy Markdown
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?

Copy link
Copy Markdown
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.

Copy link
Copy Markdown
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!

Copy link
Copy Markdown
Member

can you rebase the PR against master

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

Copy link
Copy Markdown
Contributor

@bnoordhuis ah thanks yes, meant v1.x

jimmyhsu commented Mar 3, 2015

Copy link
Copy Markdown
Contributor Author

@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 added the doc Issues and PRs related to the documentations. label Mar 22, 2015

Copy link
Copy Markdown
Contributor

ping @jimmyhsu

Copy link
Copy Markdown
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 commented Jun 25, 2015

Copy link
Copy Markdown
Member

LGTM. Landed in 54d5437.

Qard closed this Jun 25, 2015
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.


Back | FazBrowse Home | New Git URL