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

tools: add unified plugin changing links for html docs #29946

Closed

Conversation

@MarekLabuz
Copy link

MarekLabuz commented Oct 12, 2019

This commit introduces additional stage in the process of generating
html docs from markdown files. Plugin transforms links to *.md files
in the respository to links to *.html files in the online documentation.

Fixes: #28689

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

This comment has been minimized.

Copy link
@Trott

Trott Oct 12, 2019

Member

Unless the test itself is substantially copied from another file with this copyright boilerplate, the copyright boilerplate should be omitted from new files.

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Oct 12, 2019

Welcome @MarekLabuz and thanks for the pull request!

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Oct 16, 2019

@nodejs/documentation On the one hand, this will resolve the issue linked near the top. On the other hand, the whole requiring-bottom-references-that-are-unused-in-the-markdown file will be flagged by our lint rules as they currently stand and it's a bit non-intuitive.

At the moment, I'm feeling kind of -0 on it, maybe -0.5. I'd prefer something less obtrusive, more "just works". But I'm not sure we can realistically get there. I'm not concerned about broken links in the markdown-rendered docs. It would be nice if they weren't broken, but probably greater than 99% of our users read the HTML docs, not the rendered markdown in the GitHub interface or whatever.

I do appreciate the effort here and I'd like to see if there's a way to improve it a bit to something I'd be more +1 on.

@MarekLabuz

This comment has been minimized.

Copy link
Author

MarekLabuz commented Oct 16, 2019

Alright, I get it, thank you for your feedback. There is also a possibility to move these references to a separate file e.g.

{
  "synopsis": {
    "Command Line Options": "cli.html#cli_command_line_options",
    "web server": "http.html"
  }
}

However, I wonder whether it will be worth to maintain it since, as you said, the vast majority of users don't use it.

@MarekLabuz MarekLabuz requested a review from Trott Oct 24, 2019
@Trott

This comment has been minimized.

Copy link
Member

Trott commented Oct 25, 2019

I think the problem here is pretty small and probably not worth any added complexity to our setup to solve, but others might feel differently. Anyone?

@Trott

This comment has been minimized.

Copy link
Member

Trott commented Nov 6, 2019

@nodejs/collaborators This could use some reviews.

@nodejs-github-bot

This comment has been minimized.

@rvagg

This comment has been minimized.

Copy link
Member

rvagg commented Nov 6, 2019

needs a rebase I think https://ci.nodejs.org/job/node-test-commit/32422/console

I recall lots of "unified" errors in the without-ssl tests in earlier release lines, worth confirming that this passes that hurdle.

Marek Łabuz and others added 4 commits Oct 12, 2019
This commit introduces additional stage in the process of generating
html docs from markdown files. Plugin transforms links to *.md files
in the respository to links to *.html files in the online documentation.

Fixes: #28689
linking json file
@Trott Trott force-pushed the MarekLabuz:replace-docs-links-unified-plugin branch from 3d1363a to 90277e7 Nov 6, 2019
@Trott

This comment has been minimized.

Copy link
Member

Trott commented Nov 6, 2019

The rebase here seems to have gotten a bit confusing. I think I cleaned it up correctly, but please check.

@MarekLabuz

This comment has been minimized.

Copy link
Author

MarekLabuz commented Nov 7, 2019

Yes, I think it is correct, thank you.

@nodejs-github-bot

This comment has been minimized.

@rvagg

This comment has been minimized.

Copy link
Member

rvagg commented Nov 8, 2019

withoutssl passed, that's all I cared about 👍

@nodejs-github-bot

This comment has been minimized.

@addaleax

This comment has been minimized.

Copy link
Member

addaleax commented Nov 30, 2019

Landed in 62c61b7, thanks for the PR and sorry it took a while for somebody to get around to merging it!

@addaleax addaleax closed this Nov 30, 2019
addaleax added a commit that referenced this pull request Nov 30, 2019
This commit introduces additional stage in the process of generating
html docs from markdown files. Plugin transforms links to *.md files
in the respository to links to *.html files in the online documentation.

Fixes: #28689

PR-URL: #29946
Reviewed-By: Anna Henningsen <anna@addaleax.net>
addaleax added a commit that referenced this pull request Nov 30, 2019
This commit introduces additional stage in the process of generating
html docs from markdown files. Plugin transforms links to *.md files
in the respository to links to *.html files in the online documentation.

Fixes: #28689

PR-URL: #29946
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@BridgeAR BridgeAR mentioned this pull request Dec 3, 2019
targos added a commit that referenced this pull request Dec 5, 2019
This commit introduces additional stage in the process of generating
html docs from markdown files. Plugin transforms links to *.md files
in the respository to links to *.html files in the online documentation.

Fixes: #28689

PR-URL: #29946
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sebastien-Ahkrin added a commit to Sebastien-Ahkrin/node that referenced this pull request Dec 5, 2019
This commit introduces additional stage in the process of generating
html docs from markdown files. Plugin transforms links to *.md files
in the respository to links to *.html files in the online documentation.

Fixes: nodejs#28689

PR-URL: nodejs#29946
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@BethGriggs BethGriggs mentioned this pull request Dec 9, 2019
Sebastien-Ahkrin added a commit to Sebastien-Ahkrin/node that referenced this pull request Dec 11, 2019
This commit introduces additional stage in the process of generating
html docs from markdown files. Plugin transforms links to *.md files
in the respository to links to *.html files in the online documentation.

Fixes: nodejs#28689

PR-URL: nodejs#29946
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins added a commit that referenced this pull request Dec 17, 2019
This commit introduces additional stage in the process of generating
html docs from markdown files. Plugin transforms links to *.md files
in the respository to links to *.html files in the online documentation.

Fixes: #28689

PR-URL: #29946
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@BethGriggs BethGriggs mentioned this pull request Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.