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

Introduce doctoc for "Table of Content" generate #1408

Merged
merged 3 commits into from
Mar 1, 2017

Conversation

PeterDaveHello
Copy link
Collaborator

Introduce doctoc to help maintain "Table of Content", make the "Table of Content" maintenance work easier, and won't miss anything important.

Please review the change of README.markdown with whitespace ignored, could be easier to find out the real diff, you can also see there is a item missed before.

Will use it to generate "Table of Contents"
README.markdown Outdated
- [Installing nvm on Alpine Linux](#installing-nvm-on-alpine-linux)
- [Problems](#problems)
<!-- START doctoc generated TOC please keep comment here to allow auto update -->
<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
Copy link
Member

Choose a reason for hiding this comment

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

instead of this comment, could we add a travis script that reruns doctoc and fails if it generates a diff?

package.json Outdated
@@ -13,7 +13,8 @@
"test/installation": "npm run --silent test/installation/node && npm run --silent test/installation/iojs",
"test/installation/node": "shell=$(basename -- $(ps -o comm= $(ps -o ppid= -p $PPID)) | sed 's/^-//'); make TEST_SUITE=installation_node test-$shell",
"test/installation/iojs": "shell=$(basename -- $(ps -o comm= $(ps -o ppid= -p $PPID)) | sed 's/^-//'); make TEST_SUITE=installation_iojs test-$shell",
"test/sourcing": "shell=$(basename -- $(ps -o comm= $(ps -o ppid= -p $PPID)) | sed 's/^-//'); make TEST_SUITE=sourcing test-$shell"
"test/sourcing": "shell=$(basename -- $(ps -o comm= $(ps -o ppid= -p $PPID)) | sed 's/^-//'); make TEST_SUITE=sourcing test-$shell",
"doctoc": "doctoc README.markdown"
Copy link
Member

Choose a reason for hiding this comment

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

let's please add --title='## Table of Contents' which should reduce diffs (also --github, to be explicit)

README.markdown Outdated
- [Listing versions](#listing-versions)
- [.nvmrc](#nvmrc)
- [Deeper Shell Integration](#deeper-shell-integration)
- [Zsh](#zsh)
Copy link
Member

Choose a reason for hiding this comment

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

The "Z" in zsh shouldn't be capitalized here - can we update the corresponding section name so that both are always lowercase?

@PeterDaveHello
Copy link
Collaborator Author

Updated! Thanks @ljharb

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

LGTM once #1408 (comment) is addressed

@PeterDaveHello
Copy link
Collaborator Author

@ljharb test added, please take a look, thanks.

.travis.yml Outdated
@@ -11,8 +11,10 @@ addons:
- ghc
- zsh
- ksh
- nodejs
Copy link
Member

Choose a reason for hiding this comment

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

um, why would we need travis to install node from apt? it already has it from nvm. can't the script run nvm install node?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated, please help me terminated the earlier builds on Travis CI if possible, thanks.

@@ -13,6 +13,7 @@ addons:
- ksh
cache:
directories:
- $HOME/.npm
Copy link
Member

Choose a reason for hiding this comment

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

this line is no longer needed, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we can still keep it to speed up npm install doctoc

.travis.yml Outdated
- if [ -n "${SHELLCHECK-}" ]; then shellcheck -s bash nvm.sh && shellcheck -s sh nvm.sh && shellcheck -s dash nvm.sh && shellcheck -s ksh nvm.sh ; fi
- if [ -n "${SHELLCHECK-}" ]; then shellcheck -s bash install.sh bash_completion nvm-exec ; fi
- if [ -z "${SHELLCHECK-}" ]; then make TEST_SUITE=$TEST_SUITE URCHIN=/tmp/urchin/package/urchin test-$SHELL ; fi
- if [ -z "${SHELLCHECK-}" ] && [ -z "${DOCTOCCHECK-}" ] ; then make TEST_SUITE=$TEST_SUITE URCHIN=/tmp/urchin/package/urchin test-$SHELL ; fi
Copy link
Member

Choose a reason for hiding this comment

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

this should probably check if [ -n "${SHELL-}" ] && [ -n "${TEST_SUITE}" ] instead :-) mind changing this? (if this one line change is in a separate commit I can pull that in ASAP)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated!

@ljharb ljharb merged commit 286090b into nvm-sh:master Mar 1, 2017
@ljharb ljharb added informational testing Stuff related to testing nvm itself. labels Mar 1, 2017
@PeterDaveHello PeterDaveHello deleted the doctoc branch March 1, 2017 12:10
edwmurph pushed a commit to edwmurph/nvm that referenced this pull request Apr 9, 2018
[Docs] [Tests] Introduce doctoc for "Table of Contents" autogeneration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
informational testing Stuff related to testing nvm itself.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants