Skip to content
This repository has been archived by the owner on Jul 20, 2020. It is now read-only.

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

Merged
merged 1 commit into from
Jul 29, 2017

Conversation

AndrewRayCode
Copy link
Contributor

@AndrewRayCode 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 🤷
  • 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 🤷
  • Stop linking to things :D

Addresses issue firtoz#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 🤷
 - 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
   🤷
 - Stop linking to things :D
@toxicFork
Copy link
Collaborator

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

Stop linking to things :D

😅

@ashl1
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 firtoz:master Jul 29, 2017
@toxicFork
Copy link
Collaborator

Thanks a lot again for the PR :)

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants