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

docs: update GitHub deployment instructions #5888

Merged
merged 4 commits into from
Nov 7, 2021

Conversation

rootwork
Copy link
Contributor

@rootwork rootwork commented Nov 5, 2021

Motivation

Following the deployment documentation at https://docusaurus.io/docs/deployment I noticed a couple of mistakes that I had to correct before I could do a successful deployment.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

N/A (documentation only)

Related PRs

None that I could find.

Matches what is listed in the default README.md of a new Docusaurus site
documentation.yml as written fails to run because the minimum node version for Docusaurus is 14.x
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 5, 2021
@netlify
Copy link

netlify bot commented Nov 5, 2021

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: ef432fb

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61876335c3727a00076bf9f7

😎 Browse the preview: https://deploy-preview-5888--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Nov 5, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 79
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-5888--docusaurus-2.netlify.app/

@rootwork
Copy link
Contributor Author

rootwork commented Nov 5, 2021

Added one more commit that's more of a suggestion than a correction, but I think it would be helpful for folks who are less experienced to get started.

@Josh-Cena
Copy link
Collaborator

Hi, thanks for the updates

About setting USE_SSH=true though, we also have #5840 that allows GIT_USER to be unspecified; not having much experience with GH pages deployment, I understand that as GIT_USER and USE_SSH being mutually replaceable. Does that mean GH now requires SSH and the username is no longer needed whatsoever? (I am only aware of the requirement of personal access token in place of password, but not the requirement of username. I thought HTTPS would always work)

@rootwork
Copy link
Contributor Author

rootwork commented Nov 6, 2021

@Josh-Cena Hmm, I guess I'm not sure on the SSH requirement. It errored with just GIT_USER set, and then I looked at the default README.md that Docusaurus generates, which has:

$ GIT_USER=<Your GitHub username> USE_SSH=true yarn deploy

If you are using GitHub pages for hosting, this command is a convenient way to build the website and push to the gh-pages branch.

When I changed it by adding USE_SSH, it worked -- but I didn't try it without GIT_USER.

Now I have a GitHub action deploying it automatically, and it doesn't look like I can deploy at all using that command, so I'm not sure how to test this further.

But it's definitely true that the current suggested command in the documentation does not work -- so it should be changed in some way. I was just suggesting that the documentation match the readme's line.

@Josh-Cena
Copy link
Collaborator

Just checked my deployment config. I don't have USE_SSH and it looks like this:

      - run:
          name: Configure Git Credentials
          command: |
            git config --global user.email "[email protected]"
            git config --global user.name "jchen-bot"
            echo "machine github.com login jchen-bot password $GITHUB_BOT_PA_TOKEN" > ~/.netrc
      - run:
          name: Deploy to GitHub Pages
          command: yarn deploy
          environment:
            GIT_USER: jchen-bot 
            ORGANIZATION_NAME: Josh-Cena
            PROJECT_NAME: Josh-Cena.github.io
            DEPLOYMENT_BRANCH: gh-pages
            CURRENT_BRANCH: master

I don't have the bandwidth to run a deployment right now. Could you show where your deployment is failing at?

@rootwork
Copy link
Contributor Author

rootwork commented Nov 6, 2021

OK, I was able to reproduce it.

To clarify, this was when I first started using Docusaurus, and I had no deployment config, at least in terms of a GitHub action. I did have a Personal Access Token set up in GitHub. I was following along at https://docusaurus.io/docs/deployment and had the following configured within Docusaurus:

  title: 'foo',
  tagline: ' bar.',
  url: 'https://rootwork.github.io',
  baseUrl: '/',
  onBrokenLinks: 'throw',
  onBrokenMarkdownLinks: 'warn',
  favicon: 'img/favicon.ico',
  organizationName: 'rootwork', // Usually your GitHub org/user name.
  projectName: 'rootwork', // Usually your repo name.

When I ran GIT_USER=rootwork yarn deploy, as it instructs to do here, I was asked for my GitHub password. I entered it, and this was the error message:

remote: Support for password authentication was removed on August 13, 2021. Please use a personal access token instead.
remote: Please see https://github.blog/2020-12-15-token-authentication-requirements-for-git-operations/ for more information.

I already had a PAT set up, as I mentioned above. I then changed the command to GIT_USER=rootwork USE_SSH=true yarn deploy and it successfully deployed.

At the very least it seems like the documentation and the readme file should offer the same example command, even if there are multiple ways to do it.

@Josh-Cena
Copy link
Collaborator

You not only need to set up the token, you need to actually use it. When it prompts for your password, you don't actually enter the password (confusing as it sounds), but the PAT with the correct access instead.

In that case, maybe revert the SSH doc change, and add some more explanation about using PAT instead of password? It isn't wrong strictly

@Josh-Cena Josh-Cena changed the title docs: deployment corrections docs: update GitHub deployment instructions Nov 7, 2021
@Josh-Cena Josh-Cena added the pr: documentation This PR works on the website or other text documents in the repo. label Nov 7, 2021
@Josh-Cena
Copy link
Collaborator

I've made the changes myself so I can move on to work on #4409. Thanks @rootwork!

@Josh-Cena Josh-Cena merged commit effa46e into facebook:main Nov 7, 2021
@rootwork
Copy link
Contributor Author

rootwork commented Nov 7, 2021

When it prompts for your password, you don't actually enter the password (confusing as it sounds), but the PAT with the correct access instead.

Oh, that is confusing! Thanks for resolving this @Josh-Cena.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: documentation This PR works on the website or other text documents in the repo.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants