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: add example for Github Pages deployment; rewrite deployment section #4409

Merged
merged 15 commits into from
Nov 7, 2021

Conversation

polarathene
Copy link
Contributor

@polarathene polarathene commented Mar 12, 2021

Motivation

Similar to the recent PR #4263 , as a new user to both Github Actions and Docusaurus; the Github Actions deployment example seemed more complicated than it needed to be. I learned how to write a simpler deployment workflow without needing to follow additional steps of setting up an SSH key to deploy.

In that same PR it was advised that it may still be desirable to deploy to an external / remote repository instead, so I've kept the existing workflow example as is, and placed it in a 2nd tab.

  • I could also update the remote example to share more in common with the new example, such as bumping the action versions from @1 to @2?
  • I've heavily documented the example with yaml comments, if that's too much noise I can reduce or remove all of it?
  • For maintenance it'd be a bit nicer if the yaml code blocks were imported files? (not sure how well that's supported)
  • For UX, it'd be useful if the configs could be collapsed and expandable, but I'm not sure if Docusaurus has the admonition equivalent of this PyMarkdown extension details. mkdocs-material has some nice examples of this.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Docs contribution only.

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 12, 2021
@netlify
Copy link

netlify bot commented Mar 12, 2021

@netlify
Copy link

netlify bot commented Mar 12, 2021

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: 555e810

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

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

@github-actions
Copy link

github-actions bot commented Mar 12, 2021

⚡️ Lighthouse report for the changes in this PR:

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

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

@polarathene
Copy link
Contributor Author

The preview URL doesn't appear to reflect the changes in this PR?: https://deploy-preview-4409--docusaurus-2.netlify.app/classic/docs/deployment#triggering-deployment-with-github-actions

@slorber
Copy link
Collaborator

slorber commented Mar 15, 2021

Thanks!

Appears on /next: https://deploy-preview-4409--docusaurus-2.netlify.app/classic/docs/next/deployment#triggering-deployment-with-github-actions

@MatanBobi @mrousavy could you help review this workflow please?

That looks good to me

Improve some comments and clarifies the file path beyond file name alone for each config file.

Additionally removes the workflow path triggers as in practice these being updated shouldn't be triggering a re-run of the workflow again (assuming deterministic build from same input results in same output).

If there is a need for such a manual trigger of the workflow is probably a better approach. Performing a build because workflow comments were modified only would be pointless for example.
This workflow assumes your documentation resided in `documentation` branch of your repository and your [publishing source](https://help.github.com/en/github/working-with-github-pages/configuring-a-publishing-source-for-your-github-pages-site) is configured for `gh-pages` branch.
This workflow assumes your documentation resides in the `documentation` branch of your repository and your [publishing source](https://help.github.com/en/github/working-with-github-pages/configuring-a-publishing-source-for-your-github-pages-site) is configured for the `gh-pages` branch.

Below are two approaches to deploying your docs with GitHub Actions. Follow the example based on if your deployment branch (eg `gh-pages`) is in:
Copy link
Contributor

Choose a reason for hiding this comment

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

"Follow the example based on if your deployment branch..." sounds a bit odd to me, maybe rephrase it to:
"Follow the correct example whether you're deployment branch is in one of the following:"

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 went with a different wording here: fcaa277

If that's not ok, let me know and I'll change it to what you prefer 👍

Copy link
Contributor

@MatanBobi MatanBobi left a comment

Choose a reason for hiding this comment

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

This looks good to me though there are some stuff I'm not 100% familiar with (I'm not a github actions expert 😅)..
The comments are really helpful :)

@polarathene
Copy link
Contributor Author

The comments are really helpful :)

When I came across the Docusaurus deployment docs, it was my first time using Github Actions myself. I am used to using Vercel or Netlify, but the project I was contributing docs to required Github Pages deployment.

The original example config (before a recent improvement) was really intimidating and confusing compared to other deployment services, it felt very complicated.

I know mine is more verbose now, but all the additional steps aren't required (no additional links to read), it's generic, copy/paste for the most part, there isn't anything user/account specific to it, nor third-party dependency like requiring generation of keys/tokens.

I wish as such a user, that I had the workflow documented like this when I first came across it, instead of having to jump across the official docs and experiment. So keeping the comments is acceptable? (with syntax highlighting it shouldn't be a problem for experienced Github Actions Workflow users)


This looks good to me though there are some stuff I'm not 100% familiar with

I'm happy to answer any questions about it that I can! 😁

Is there a way for Docusaurus to import these config files separately for maintenance?

Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

That looks way too verbose for me

Users using Docusaurus want to get their pages online asap, not become GH actions CI experts and read so much text to figure out all the edge cases and config options.

I'm not against having a more exhaustive doc, but it shuld not be the first things most users see, and hidden in another tab or something imho

website/docs/deployment.mdx Outdated Show resolved Hide resolved
# https://github.com/actions/checkout/issues/13#issuecomment-724415212
# https://api.github.com/users/github-actions%5Bbot%5D
GIT_USER: 'github-actions[bot]'
GIT_EMAIL: '41898282+github-actions[bot]@users.noreply.github.com'
Copy link
Collaborator

Choose a reason for hiding this comment

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

where does this email come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It comes from where the comments above it clearly state (See the referenced github issue](actions/checkout#13 (comment))).

I had the same question when I was trying to resolve this properly and had to seek that information out.

There is a user and bot account, the ID value can be verified via the Github API, the e-mail matches what Github provides every user in their settings page to use instead of a personal e-mail:

website/docs/deployment.mdx Outdated Show resolved Hide resolved
@slorber slorber mentioned this pull request Mar 19, 2021
Upstream doesn't see value including help comments for a copy/paste config under the basis that it adds friction to the viewer seeking guidance how to perform something they don't know.
@polarathene
Copy link
Contributor Author

Users using Docusaurus want to get their pages online asap

  1. Users that want to get the CI setup working ASAP are going to copy/paste or glance over the syntax highlighted config file.
  2. They either understand the contents and comments being syntax highlighted are ignored, they verify everything looks good and apply it.
  3. Or they don't understand it, blindly apply it or try to understand it like I did by jumping around many GH docs pages and doing multiple queries on Google when there was a lack of clarity/information on the subject for certain parts.

Personally, I see no harm or friction added by the helpful contextual comments, especially on a page providing guidance on the config which implies that the reader seeking this information may benefit from that information 🤷‍♂️

not become GH actions CI experts and read so much text to figure out all the edge cases and config options.

I do not cover so many edge cases and config options. There's plenty more for this CI service, I just focused on what mattered for Docusaurus. It's what I wish Docusaurus had initially when I came across the docs pages (especially before the recent improvement on this deploy config), I had used other services but not GH Pages.

I'm not against having a more exhaustive doc, but it shuld not be the first things most users see, and hidden in another tab or something imho

Verbosity under this context is not a bad thing and I doubt it contributes to friction, if anything it does quite the opposite. This is a page the user is ideally only needing to view content for once, it's an important step to document well, it's important for maintainers of the configs that have already cited a lack of experience and familiarity with it, they shouldn't have to go through the effort I did if it could all be inlined for everyones benefit like it was.

Regardless, I've removed the comments as requested. I did not duplicate the prior config with comments to an additional tab, that would likely negatively impact the UX for the user (value of the comments aside) and add to maintenance.

MatanBobi requested a rephrase during review.
@sagikazarmark
Copy link

My two cents: when I looked at the documentation for GitHub Actions, I was surprised that I had to manually setup a deploy key, just to use the builtin deploy tool. In the end, I used peaceiris/actions-gh-pages, because I used it before.

So I'd say anything that removes a manual step from the process is a step forward. I agree that users want to get their sites deployed and they don't care if it happens using the builtin deploy script or not.

I somewhat agree with the verbosity argument, but it's orthogonal to the deployment tool decision: as a new user, I wouldn't care about caching or setting an alternate git user, so those should probably be moved out to an advanced example.

Full disclosure: I'm in fact a new user who deployed their first docusaurus site today.

If it's any help: here is the config I ended up with: https://github.com/twirphp/twirphp.github.io/blob/main/.github/workflows/deploy.yaml

@Josh-Cena
Copy link
Collaborator

FWIW, I use peaceiris/actions-gh-pages to deploy my sites as well—it masks away all the details of deployment. Had everyone started by using that, I doubt if all these explanations are useful.

On top of that, I do have some strong feelings towards exhaustively documenting the deployment process. We had closed down a few other PRs introducing new deployment options in the past few months. Still, GH pages remains as one of the most competitive options despite its extreme difficulty and limitation in configuration, so we may be biased and provide more writeup for it.

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Nov 1, 2021

Also, we now do have the <details /> comp, which can be useful if you want to introduce some technical details: https://docusaurus.io/docs/next/markdown-features/plugins#installing-plugins

For maintenance it'd be a bit nicer if the yaml code blocks were imported files? (not sure how well that's supported)

This is also doable.

```mdx-code-block
import CodeBlock from '@theme/CodeBlock';
<CodeBlock className="language-js" title="sidebars.js">
{require('!!raw-loader!@site/sidebars.js')
.default
.split('\n')
// remove comments
.map((line) => !['#','/*','*'].some(commentPattern => line.trim().startsWith(commentPattern)) && line)
.filter(Boolean)
.join('\n')}
</CodeBlock>
```

@polarathene
Copy link
Contributor Author

polarathene commented Nov 1, 2021

On my end, the project I migrated Github Wiki docs for the decision went with mkdocs-material instead of docusaurus (as much as I liked it).

Note: Github Actions specific, not docusaurus specific:

Deployment became a bit more difficult with integrating preview builds for PRs as Github Actions security policies make that complicated (requires a separate build workflow and deploy workflow), although if you don't need to support deploy previews for PRs from untrusted contributors it's a non-issue. (this was also because we did this all via GH actions, even though previews are deployed to Netlify, as we are an organization and didn't want to incur billing if CI exceeded build minute limits)


I wanted to resolve this PR but didn't receive continued review feedback. I'm not able to justify allocating the time to participate further in the discussion/review to get it into an acceptable state for merging unfortunately. If there is still interest in it, hopefully someone can take over :)

I still stand by my reasons for verbosity being a positive and non-issue in practice given the context of the material and reader.

@Josh-Cena
Copy link
Collaborator

I wanted to resolve this PR but didn't receive continued review feedback. I'm not able to justify allocating the time to participate further in the discussion/review to get it into an acceptable state for merging unfortunately. If there is still interest in it, hopefully someone can take over :)

Sure, I would be taking it over 👍

I do agree that we can have some explanations, probably in <details />

@polarathene
Copy link
Contributor Author

Sure, I would be taking it over

That'd be much appreciated! Thank you 👍

@slorber
Copy link
Collaborator

slorber commented Nov 3, 2021

Thanks @Josh-Cena

I don't have much experience with GH pages, existing actions ecosystem so it's time consuming to review this doc atm.

We could probably restore our own GH action to deploy our site on a test subdomain, just for dogfooding (as an e2e test for the deploy command). https://github.com/facebook/docusaurus/tree/gh-pages

@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 Josh-Cena changed the title docs: Improve the deploy example for Github Pages docs: add example for Github Pages deployment; rewrite deployment section Nov 7, 2021
@Josh-Cena
Copy link
Collaborator

This is in a good shape to merge. Made some polishings to the deploy docs in general.

@Josh-Cena Josh-Cena merged commit caa9d92 into facebook:main Nov 7, 2021
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.

6 participants