-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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: fixed 404s #409
doc: fixed 404s #409
Conversation
I didn't test it myself either, but it's trivial 404 fixing and this PR fixes them. +1 |
This should not be merged as-is. Instead, these files should either move down a directory into the API assets or be made available by the website. These files provide syntax highlighting for the docs. |
The sh.css file isn't made available by the website. So I just moved the JS files into the same dir as that file. Is this not correct? |
Hurk, that's what I get for trying to review on a phone screen. |
website_files = \ | ||
out/doc/sh_main.js \ | ||
out/doc/sh_javascript.min.js | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't the path on these files change, instead of deleting the variable entirely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did that and got:
cp -r doc/api_assets/sh_main.js out/doc/api_assets/sh_main.js
cp: out/doc/api_assets/sh_main.js: No such file or directory
make: *** [out/doc/api_assets/sh_main.js] Error 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the directory would be out/doc/assets/
-- it's rewritten to drop the leading api_
, which is kind of odd.
Updated Makefile to remove special casing for those files, and moved the files to doc/api_assets. Fixes: nodejs/iojs.org#51 PR-URL: #409 Reviewed-By: Chris Dickinson <[email protected]>
Merged in e789103. Thanks! |
Also changed URL for font to use current protocol rather than forcing protocol.
Haven't tested this works, will check when I get on my dev machine.
See nodejs/iojs.org#51 for 404 issue