-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Website: fix version display #582
Conversation
I want to discuss doc duplication with you. I feel uncomfortable duping all docs 3 times (and potentially even more in the future). can't we generate the docs per version via git tags only on gh-pages branch during build ? |
517b7ff
to
76e8698
Compare
@@ -52,6 +52,12 @@ matrix: | |||
|
|||
# Website | |||
- language: node_js | |||
addons: |
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.
This is needed by nodegit
@rotemmiz I thought you were not sure about storing them there, I didn't understand earlier that this is really a problem for you. The way I solved it now is by getting every git tag above version 6 (where we changed the docs to support docusaurus) and generate the version for each doc in a temporary copy of this repo, which is then copied over. This is currently WIP, the sidebars don't work in the current implementation, need to figure out what's going wrong there 🤔 Is this the general direction you want to go with this or did I misunderstand you? 😇 |
74e6108
to
584faa3
Compare
5e3ff4d
to
1df93a1
Compare
1df93a1
to
9307cb7
Compare
@rotemmiz This is done now / ready for review. I would like to move the code upstream to docusaurus, I think it is a better place for it. Easier to test and be concise this way (changing docusaurus versions could break this current workflow if they change something severe in the way they deal with versions). This is the issue I created for it: facebook/docusaurus#557 |
9307cb7
to
c472253
Compare
website/gatherDocs.js
Outdated
}); | ||
} | ||
|
||
function getMajorVersion(tag) { |
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.
Please use 'semver' instead of parsing versions manually, this will break the pretty easily
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.
Good idea 👍
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 fixed that @rotemmiz 👍 Please review again!
e7c2773
to
cc2b436
Compare
@rotemmiz This is ready now 👍 I only clone the repo once and execute every action on this copy now :) |
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.
Great PR!
Have very few small notes.
website/gatherDocs.js
Outdated
await repo.checkoutBranch(version); | ||
} | ||
|
||
function applyTemporalFix(tempDir) { |
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.
applyMissingDocHeadersForV6
or whatever version which needs it.
website/pages/en/versions.js
Outdated
@@ -0,0 +1,90 @@ | |||
/** | |||
* Copyright (c) 2017-present, Facebook, Inc. |
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.
Is this Facebook's code ?
If not, remove the license note
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.
It's generated by docusaurus, should I remove this part?
|
||
function VersionLinks({version, isLatest = false}) { | ||
const expandedVersion = version.replace('.X', '.0.0') | ||
const isMaster = version === "master"; |
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.
Not sure I get what the final result will be. Will site show exact version (i.e. 7.3.5) or 7.0.0 throughout links ?
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.
Video shows 7.X, 6.X and master. what am I missing ?
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.
Video is old, but you can run it locally. Just do cd website/ && npm install && npm run gatherDocs && npm start
and you can see the site
@@ -1,110 +0,0 @@ | |||
--- |
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.
<3<3<3<3<3<3<3<3<3<3<3<3<3<3
Damn it, the checkout seems to have failed, unnoticed by me. Need to do some more node-git magic here. |
@rotemmiz I think I got your review comments, please take a look 👍 |
website/gatherDocs.js
Outdated
} | ||
|
||
// https://stackoverflow.com/questions/29569913/switch-branch-tag-with-nodegit?utm_medium=organic&utm_source=google_rich_qa&utm_campaign=google_rich_qa | ||
function checkOutTag(repo, tag) { |
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.
LOL, utm_source will provide false data for trackers next time someone opens this link ;p
JK, but the query string can be removed
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.
Right, I'll get rid of that and see if I can get a better permalink
@rotemmiz Got a spare review? |
This PR has two different aspects, one is "releasing" the 7.X version of the docs.
The other one is having a versions page so that when you click on the current version on the top left. Here is a video of this: