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

Fixing (hopefully) all three.js documentation links #177

Merged
merged 1 commit into from Jul 29, 2017

Conversation

@AndrewRayCode
Copy link
Contributor

AndrewRayCode commented May 10, 2017

Addresses issue #175

Here's how this was done:

First I extracted all links using Ack and Vim, and removed duplicates using :sort u in Vim.

Through sheer luck I found all three.js doc pages load https://threejs.org/docs/list.js, which lists all relative doc paths, excluding method links.

Then I wrote a node script to compare all links found in the R3R project to the links from the above mentioned list.js. That script is found here

If you run this script with node test-r3r-links.js, it will not error. The assert on the last line is what verifies all links in R3R source exist in three.js's list.js index.

NOTE: I DID NOT HAND VERIFY THE METHOD SPECIFIC LINKS. As in, I know https://threejs.org/docs/#api/extras/core/Path is correct, but I did not hand verify all the method links like https://threejs.org/docs/#api/extras/core/Path.absarc

The actual change to the codebase took several search and replace sweeps in Visual Studio Code. It was made easier by the fact that the original doc links in R3R link to http, while the updated three docs all redirect to https.

Fun facts:

  • This was not a simple search and replace. Some things moved, for example Extras.Geometries moved to api/core/geometries
  • There were some links accidentally to index.html#/api which required specific hand hold replacing
  • New doc links have lowercase, /materials/ vs /Materials/
  • I considered making a quick scraper to make sure all replaced links were valid, but because threejs uses iframes, all broken links return 200s :shrug:
  • Going to the actual doc page in the iframes redirect to broken URLs. For example, try going to https://threejs.org/docs/api/textures/Texture :shrug:
  • Stop linking to things :D
Addresses issue #175

Here's how this was done:

First I extracted all links using Ack and Vim, and removed duplicates
using `:sort u` in Vim.

Through sheer luck I found all three.js doc pages load
[https://threejs.org/docs/list.js](https://threejs.org/docs/list.js), which
lists all relative doc paths, excluding method links.

Then I wrote a node script to compare all links found in the R3R project
to the links from the above mentioned `list.js`. [That script is found here](https://gist.github.com/AndrewRayCode/407067e1fe739bac7fae7e9ef295111e)

If you run this script with `node test-r3r-links.js`, it will not error.
The assert on the last line is what verifies all links in R3R source
exist in three.js's list.js index.

NOTE: I DID NOT HAND VERIFY THE METHOD SPECIFIC LINKS. As in, I know
https://threejs.org/docs/#api/extras/core/Path is correct, but I did not
hand verify all the method links like https://threejs.org/docs/#api/extras/core/Path.absarc

The actual change to the codebase took several search and replace sweeps
in Visual Studio Code. It was made easier by the fact that the original
doc links in R3R link to http, while the updated three docs all redirect
to https.

Fun facts:
 - This was not a simple search and replace. Some things moved, for
   example Extras.Geometries moved to api/core/geometries
 - There were some links accidentally to index.html#/api which required
   specific hand hold replacing
 - New doc links have lowercase, `/materials/` vs `/Materials/`
 - I considered making a quick scraper to make sure all replaced links
   were valid, but because threejs uses iframes, all broken links return
   200s :shrug:
 - Going to the actual doc page *in* the iframes redirect to broken
   URLs. For example, try going to https://threejs.org/docs/api/textures/Texture
   :shrug:
 - Stop linking to things :D
@toxicFork

This comment has been minimized.

Copy link
Owner

toxicFork commented May 14, 2017

Sweet! Thanks!! I'll verify and merge ASAP!

Stop linking to things :D

😅

@ashl1

This comment has been minimized.

Copy link

ashl1 commented Jul 18, 2017

Any updates on this? Merging the PR will improve docs expirience for the react-three-render novices.

@toxicFork toxicFork merged commit e60283c into toxicFork:master Jul 29, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@toxicFork

This comment has been minimized.

Copy link
Owner

toxicFork commented Jul 29, 2017

Thanks a lot again for the PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.