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: documented make docopen #19321

Conversation

AyushG3112
Copy link
Contributor

@AyushG3112 AyushG3112 commented Mar 13, 2018

Documented make docopen as a way to read documentation in the browser.

Checklist

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels Mar 13, 2018
@addaleax
Copy link
Member

Fwiw, you don’t need a webserver to read the docs, just opening the generated HTML files in your browser works.

@AyushG3112
Copy link
Contributor Author

@addaleax true, and perhaps I should have been more explicit in the description.

But personally as a user, I still prefer the SimpleHTTPServer way because if I close my browser, I do not have to navigate the HTML files to open the docs again, but that's just me.

@addaleax
Copy link
Member

@AyushG3112 Yeah, not telling you what to do :) It might be nice to document make docopen too though, it does basically this.

@AyushG3112
Copy link
Contributor Author

@addaleax oh, I did not know about make docopen.

Should I document it in this PR, because if I do the title of the PR would not be enough?
Also, If I do document make docopen, should I add it in addition to the SimpleHTTPServer or replace the SimpleHTTPServer with it?

@addaleax
Copy link
Member

@AyushG3112 All of that is completely up to you. :)

Should I document it in this PR, because if I do the title of the PR would not be enough?

You can always change a PR title, and if you use git commit --amend (and git push --force-with-lease instead of git push), you can also change the commit message & contents of an existing commit.

Let us know if you need any help!

@AyushG3112 AyushG3112 changed the title doc: Added python SimpleHTTPServer as a method reading docs in browser doc: documented make doconly Mar 13, 2018
@AyushG3112
Copy link
Contributor Author

AyushG3112 commented Mar 13, 2018

@addaleax Changed :) . Could you let me know if any improvement is needed? First time doc-only contribution to Node.JS here

@addaleax
Copy link
Member

This seems perfectly fine to me. :)

Light CI: https://ci.nodejs.org/job/node-test-pull-request-lite/256/

@vsemozhetbyt
Copy link
Contributor

Sorry, I cannot find any doconly task in Makefile. Should it be docopen?

node/Makefile

Lines 672 to 674 in 52e869b

.PHONY: docopen
docopen: $(apidocs_html)
@$(PYTHON) -mwebbrowser file://$(PWD)/out/doc/api/all.html

@AyushG3112 AyushG3112 changed the title doc: documented make doconly doc: documented make docopen Mar 13, 2018
@vsemozhetbyt
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt added the fast-track PRs that do not need to wait for 48 hours to land. label Mar 13, 2018
BUILDING.md Outdated
@@ -206,6 +206,15 @@ To read the documentation:
$ man doc/node.1
```

If you prefer to read the documentation in the browser,
Copy link
Member

Choose a reason for hiding this comment

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

Tiny optional nit: in the browser -> in a browser

BUILDING.md Outdated
$ make docopen
```

This will open up a browser tab/window with the documentation.
Copy link
Member

Choose a reason for hiding this comment

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

Optional nit: open up a browser tab/window -> open a browser

BUILDING.md Outdated
@@ -206,6 +206,15 @@ To read the documentation:
$ man doc/node.1
```

If you prefer to read the documentation in the browser,
run the following once `make doc` is done:
Copy link
Member

Choose a reason for hiding this comment

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

Optional nits: once -> after and done -> finished

@AyushG3112
Copy link
Contributor Author

@Trott updated. Also, I rebased to master before pushing, which is the reason for increased number of changes.

@AyushG3112
Copy link
Contributor Author

@addaleax can this land?

Copy link
Member

@trivikr trivikr left a comment

Choose a reason for hiding this comment

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

Confirmed that comments of @Trott were incorporated and make docopen works as described.
This PR is good to land.

@trivikr
Copy link
Member

trivikr commented Mar 26, 2018

Documented an easier way for users to browse documentation without
setting up a whole web environment. It make the process of reading the
latest documentation, and comparing it with the documentation of
older versions on the Node.JS website a lot easier.
Documented `make doconly` as a way to read documentation in the browser.
Documented an easier way for users to browse documentation without
setting up a whole web environment. It make the process of reading the
latest documentation, and comparing it with the documentation of
older versions on the Node.JS website a lot easier.
Documented `make doconly` as a way to read documentation in the browser.
@AyushG3112 AyushG3112 force-pushed the building-docs-python-simplehttpserver branch from 2afa7b1 to f6a70dc Compare March 26, 2018 04:50
@AyushG3112
Copy link
Contributor Author

@trivikr Rebased.

@trivikr
Copy link
Member

trivikr commented Mar 26, 2018

@trivikr
Copy link
Member

trivikr commented Mar 26, 2018

Rerun of CI-lite was successful https://ci.nodejs.org/job/node-test-pull-request-lite/338/

@trivikr
Copy link
Member

trivikr commented Mar 26, 2018

Landed in 0d5720b

@trivikr trivikr closed this Mar 26, 2018
trivikr pushed a commit that referenced this pull request Mar 26, 2018
Documented `make docopen` as a way to read documentation in the browser.

PR-URL: #19321
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos pushed a commit that referenced this pull request Mar 27, 2018
Documented `make docopen` as a way to read documentation in the browser.

PR-URL: #19321
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
BethGriggs pushed a commit that referenced this pull request Dec 4, 2018
Documented `make docopen` as a way to read documentation in the browser.

PR-URL: #19321
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
@BethGriggs BethGriggs mentioned this pull request Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. fast-track PRs that do not need to wait for 48 hours to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants