Skip to content

Conversation

@srcmake
Copy link
Contributor

@srcmake srcmake commented Jul 6, 2019

I was looking at synopsis.md and noticed that the links in the file are broken. Basically, they point to .html files instead of .md files. This PR fixes it.

Testing:
In the node repository's synopsis.md:

  1. Click the "Command Line Options" link. Notice it takes you to a 404 page.
  2. Click the "web server" link. Notice it takes you to a 404 page.

In my branch's synopsis.md

  1. Click the "Command Line Options" link. Notice it takes you to the correct page.
  2. Click the "web server" link. Notice it takes you to the correct page.
Checklist

The links in synopsis.md were pointing pages with
.html extensions. The correct extension is .md for
those pages. This commit fixes those links.
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jul 6, 2019
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request. This will break the links on the website. The md files are inputs for generating html files on the website. It’s imperfect that the links don’t work in the source tree but it’s the better trade off.

If you want the links to be correct in both places, you’ll need to update the doc-generation script in the tools directory to intelligently update the links.

@srcmake
Copy link
Contributor Author

srcmake commented Jul 6, 2019

Thanks for the pull request. This will break the links on the website. The md files are inputs for generating html files on the website. It’s imperfect that the links don’t work in the source tree but it’s the better trade off.

If you want the links to be correct in both places, you’ll need to update the doc-generation script in the tools directory to intelligently update the links.

Ohhhhh. I was wondering why they were .html after looking at your PR and was going to comment there, but I figured I'd just make the change myself...I didn't realize those files were used to generate the website. (Thanks for telling me, I always wondered where that website came from.)

In that case, I'm sure there are a lot of other links that point to .html files, and not just the ones in this file. So this PR isn't even that helpful

I think the correct steps forward is to create a new issue (to possibly make the generate.js tool smarter, citing this "viewing these files on github doesn't link nicely since the extensions are incorrect") for this and close this PR. Does that sound fine? If so, I can make the issue.

@Trott
Copy link
Member

Trott commented Jul 6, 2019

I think the correct steps forward is to create a new issue (to possibly make the generate.js tool smarter, citing this "viewing these files on github doesn't link nicely since the extensions are incorrect") for this and close this PR. Does that sound fine? If so, I can make the issue.

Works for me.

(Only the docs section of the website is generated from those md files. The rest of the website is in the https://github.com/nodejs/nodejs.org repository.)

Copy link
Contributor

@Fishrock123 Fishrock123 left a comment

Choose a reason for hiding this comment

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

Unfortunately as Rich said, this needs to be kept for the website. Which is... more important that local links.

If we can build around this somehow that would be fine but should probably be done in a different PR.

@srcmake
Copy link
Contributor Author

srcmake commented Jul 15, 2019

I made an Issue for this here: #28689

I'm closing this PR. Thanks for the advice Trott and Fishrock.

@srcmake srcmake closed this Jul 15, 2019
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.

4 participants