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

Update website to use Docusaurus #4536

Merged
merged 18 commits into from
Oct 11, 2017
Merged

Update website to use Docusaurus #4536

merged 18 commits into from
Oct 11, 2017

Conversation

ericnakagawa
Copy link
Contributor

Summary

I have done a complete conversion of Jest's website to use Docusaurus.

Docusaurus provides all of the same features: crowdin integration, static page building, deployment to github, and algolia search.

@ericnakagawa
Copy link
Contributor Author

I know what the error is. I'll resolve and send an update to this this PR.

The issue is the 'yarn build' script tries building the website, but looks for localized files and fails if they are missing.

@codecov-io
Copy link

codecov-io commented Sep 26, 2017

Codecov Report

Merging #4536 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4536   +/-   ##
=======================================
  Coverage   57.13%   57.13%           
=======================================
  Files         194      194           
  Lines        6565     6565           
  Branches        3        3           
=======================================
  Hits         3751     3751           
  Misses       2814     2814

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5049651...0f0d6a2. Read the comment docs.

@@ -27,15 +27,18 @@ aliases:
git config --global user.email "jest-bot@users.noreply.github.com"
git config --global user.name "Website Deployment Script"
echo "machine github.com login jest-bot password $GITHUB_TOKEN" > ~/.netrc
# crowdin install start
# install Docusaurus and generate file of English strings
cd website && npm install && npm run write-translations && cd ..
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we install the deps w yarn instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heck yeah! I'll swap that out now.

@ericnakagawa
Copy link
Contributor Author

@thymikee Hi Mike, are you able to review and accept?

@thymikee
Copy link
Collaborator

I'll see what I can do!

"crowdin-upload": "crowdin-cli --config ../crowdin.yaml upload sources --auto-update -b master",
"crowdin-download": "crowdin-cli --config ../crowdin.yaml download -b master"
"crowdin-download": "crowdin-cli --config ../crowdin.yaml download -b master",
Copy link
Collaborator

@thymikee thymikee Sep 26, 2017

Choose a reason for hiding this comment

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

I don't see crowdin-download nor crowdin-upload used anywhere – is this expected and somehow used implicitly?

Also, README.md would use an update (e.g. port is outdated)

Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

It's hard for me to test it, because I'm not sure how to fully set this up locally – I don't have access to config keys, which makes all requests to /en et al. crash :<


const translate = require('../../server/translate.js').translate;

// const siteConfig = require(process.cwd() + "/siteConfig.js");
Copy link
Collaborator

Choose a reason for hiding this comment

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

dead code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

@@ -1,94 +0,0 @@
/* eslint-disable sort-keys */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aren't we providing RRS feed anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Docusaurus doesn't have an RSS feature. I've made it a TODO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New PR includes updated Docusaurus with RSS/Atom feeds.

const Container = CompLibrary.Container;
const GridBlock = CompLibrary.GridBlock;

const translate = require('../../server/translate.js').translate;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this file generated somehow? It's deleted in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a file from Docusaurus.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok. I'd expect something like:

const {translate} = require('docusaurus/server');

but not sure how this thing works currently.

* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

License headers need to be adjusted to MIT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't adjust any additional licenses at this time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thymikee Great news Michał! Docusaurus is now licensed under MIT!

Copy link
Member

Choose a reason for hiding this comment

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

Still bsd here though

ogImage: 'img/opengraph.png',
recruitingLink: 'https://crowdin.com/project/jest',
algolia: {
apiKey: '833906d7486e4059359fa58823c4ef56',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we expose Algolia API key?

Copy link
Contributor Author

@ericnakagawa ericnakagawa Sep 26, 2017

Choose a reason for hiding this comment

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

Good point. This would be the same issue as you not having crowdin keys to test. I'll move the keys to Circle CI.

}

@media only screen and (min-width: 1500px) {
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused media queries

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed them.

@orta
Copy link
Member

orta commented Sep 28, 2017

Impressive amount of deletions here 👍

@ericnakagawa
Copy link
Contributor Author

@thymikee Please do another review. Hoping this update meets the expected standards! Preview -- https://build-mitsvvnzqs.now.sh/jest#

@orta
Copy link
Member

orta commented Oct 6, 2017

Not sure if this is related, but the footer is using the wrong repo for the stars:

screen shot 2017-10-06 at 1 45 56 pm

@thymikee
Copy link
Collaborator

thymikee commented Oct 6, 2017

@ericnakagawa I'm out for the weekend, but if I make some time, definitely have a look. Can I somehow build it locally?

@ericnakagawa
Copy link
Contributor Author

ericnakagawa commented Oct 10, 2017

@orta Thanks for the catch. Latest push has matching footer info + logos.
@thymikee You can manually run by running yarn from website, but you won't be able to sync with/to crowdin since that requires having Jest's crowdin keys.

Here's a fresh build from my machine: https://build-qbmkjpokcg.now.sh (currently uploading on airplane WiFi)

@@ -0,0 +1,11 @@
node_modules
Copy link
Member

Choose a reason for hiding this comment

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

why a nested gitignore?

@SimenB
Copy link
Member

SimenB commented Oct 10, 2017

image

Loving it 😀

Only issue I found (other than the footer, which is fixed in latest commit) is that all the "Edit this doc" links 404 on github (they are missing the language). That might be fixed on the latest upload, it currently gives me 403.

One other thing (not sure if issue or not) is that all the links in the sidebar includes the hash #content. Doesn't seem to affect anything, but is sorta weird

@@ -1,4 +1,3 @@
node_modules
Copy link
Member

Choose a reason for hiding this comment

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

I meant the whole file, why not just stick the ignores you need in the root .gitignore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@SimenB Ahh, I see. In event we ever break out the website into its own repo. For time being, I'll migrate the paths to the top level .gitignore.

@ericnakagawa
Copy link
Contributor Author

@SimenB I removed the line from .gitignore and updated the edit doc path to include /en.

As for the sidebar links going to #content I've added this as an issue on the Docusaurus repo -- I'll review and fix there, then update in Jest once we decide. facebook/docusaurus#120

@ericnakagawa
Copy link
Contributor Author

Due to my current internet connection I am unable to troubleshoot the breaking CI test.

@SimenB
Copy link
Member

SimenB commented Oct 10, 2017

The CI failure is just prettier it seems like. So a yarn lint --fix should make everything nice and happy

@ericnakagawa
Copy link
Contributor Author

@thymikee @SimenB Requesting a review! I had to manually re-run a job because of a segfault.

@SimenB
Copy link
Member

SimenB commented Oct 10, 2017

Only thing I could spot was the comment about the license, nothing else I noticed (either in the code or clicking through https://build-qbmkjpokcg.now.sh/jest)

@ericnakagawa
Copy link
Contributor Author

@SimenB Updated last file, also updated package.json to use latest docusaurus with MIT license throughout.

image: '/jest/img/logos/facebook.png',
infoLink: 'https://code.facebook.com',
pinned: true,
caption: "Facebook",
Copy link
Member

Choose a reason for hiding this comment

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

your autoformat kicked in, I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

lol

@ericnakagawa
Copy link
Contributor Author

Rebased and waiting for tests.

Copy link
Collaborator

@thymikee thymikee left a comment

Choose a reason for hiding this comment

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

looks good 👍

@SimenB
Copy link
Member

SimenB commented Oct 10, 2017

Seeing as this PR moves all files out of en directory, I'm starting to wonder if 1fc503b#diff-c272134cac3612b41f93893bbaa34d24R254 was a mistake?

Updating sidebar

Modifying files into Docusaurus format

Slight header logo CSS change

Ignore translated files and build directory

Proper file locations for Docusaurus

Move blog files to Docusaurus preferred location, modified crowdin to output /docs to /translated_pages, updated siteConfig.js

Eslinting Docusaurus files

Ignoring website/blog pages

Linter fixes
@ericnakagawa
Copy link
Contributor Author

Rebased again after docs updated.

@ericnakagawa
Copy link
Contributor Author

@SimenB Ah, I knew I had a reason for configuring it this way. I've rolled back the change.

Rebased, and ran through lint fixer.

We should be good to go!

@ericnakagawa ericnakagawa merged commit 172c0b2 into jestjs:master Oct 11, 2017
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants