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

test: use addon.md block headings as test dir names #4412

Closed
wants to merge 0 commits into from

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Dec 24, 2015

Following on from #4411 (includes the commit from there), but not urgent and not absolutely necessary. Instead of naming the directories doc-X where X is an incrementing number, name them according to the section of the doc from which they come so when debugging you have a clue what it's doing! Requires touch a lot of stuff, I hope I'm not missing anything.

/cc @nodejs/build

@rvagg
Copy link
Member Author

rvagg commented Dec 24, 2015

@mscdex mscdex added c++ Issues and PRs that require attention from people who are familiar with C++. doc Issues and PRs related to the documentations. test Issues and PRs related to the tests. labels Dec 24, 2015
@rvagg
Copy link
Member Author

rvagg commented Jan 13, 2016

ping @nodejs/build, @nodejs/testing any thoughts on this?

@jasnell
Copy link
Member

jasnell commented Jan 13, 2016

LGTM but would like to have at least @jbergstroem and @Trott take a look

@Trott
Copy link
Member

Trott commented Jan 14, 2016

Semi-rubber-stamp LGTM

if (token.type === 'heading') {
if (token.type === 'heading' && token.text) {
blockName = token.text
console.log(token)
Copy link
Member

Choose a reason for hiding this comment

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

unnecessary?

@jbergstroem
Copy link
Member

LGTM with optional bonus nit.

rvagg added a commit that referenced this pull request Jan 14, 2016
instead of doc-*

PR-URL: #4412
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
@rvagg rvagg closed this Jan 14, 2016
@rvagg rvagg force-pushed the name-addon.md-test-directories branch from 44a666b to 3727ae0 Compare January 14, 2016 11:05
@rvagg
Copy link
Member Author

rvagg commented Jan 14, 2016

landed @ 3727ae0, thanks folks

rvagg added a commit that referenced this pull request Jan 14, 2016
instead of doc-*

PR-URL: #4412
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 28, 2016
instead of doc-*

PR-URL: #4412
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 11, 2016
instead of doc-*

PR-URL: #4412
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 11, 2016
instead of doc-*

PR-URL: nodejs#4412
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 11, 2016
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Feb 15, 2016
instead of doc-*

PR-URL: nodejs#4412
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
instead of doc-*

PR-URL: nodejs#4412
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Johan Bergström <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. doc Issues and PRs related to the documentations. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants