-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
feat(v2): Improve the initial templates #4302 #4320
feat(v2): Improve the initial templates #4302 #4320
Conversation
[pull] master from facebook:master
[pull] master from facebook:master
[pull] master from facebook:master
[V1] Deploy preview success Built with commit 4eb31e4 |
Size Change: -46 B (0%) Total Size: 532 kB
ℹ️ View Unchanged
|
Deploy preview for docusaurus-2 ready! Built with commit 4eb31e4 |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4320--docusaurus-2.netlify.app/classic/ |
@slorber Still waiting for a review |
# Conflicts: # packages/docusaurus-init/templates/classic/docs/create-a-page.md # packages/docusaurus-init/templates/classic/docs/getting-started.md # packages/docusaurus-init/templates/classic/docs/markdown-features.mdx # packages/docusaurus-init/templates/classic/docs/thank-you.md # packages/docusaurus-init/templates/classic/sidebars.js
FYI I merged master in this PR and resolved the conflicts for you |
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.
Same comment as on the PR by @ShinteiMai
This page is way too verbose, we need something much more simple.
We don't need an exhaustive guide on how to deploy to GH pages, and particularly not a tutorial on how to automate this process with Github actions and the complicated CI script. This does not really spark joy to the user but rather a headache 😅
Basically, you just need to tell the user to run npm run build
, and document the simplest way to get this site online.
- Self-host:
npm run serve
(no need to explain --port -- host options here or tell it's not performant) - Github pages: tell the user to add a minimal config and
npm run deploy
(don't show an exhaustive list of env variable options here, don't tell how to automate this) - Netlify or Vercel: signup, run
npm run build
and then drag & drop the ./build folder
Try to remove useless wordings and keep it minimalistic, inspire yourself from the 1st part which was largely trimmed, and use an imperative tone.
We should rather tell the user exactly what to do, we should not really give him any options (apart from choosing a hosting solution), so avoid telling the user about any cli or env variable options
@slorber, I have made some changes |
Hey @besemuna . Can you add a few screen shots for easy reviewing of your new changes. We will have lots of positive feedback if a screenshot is available. |
@javidrashid I have added a screen recording. |
Thanks, that looks better to me Can you please run prettier on your docs files? Those are not formatted and there are missing line breaks around code blocks/admonitions. I'm even surprised that the CI linter does not complain on this, will investigate |
It is not the most performant solution | ||
::: | ||
|
||
:::warning It is not the most performant solution ::: |
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.
unfortunately this code is not valid anymore after prettier has been run 😅 that's why I said there were missing line breaks, because you have to add extra line breaks in some places before running prettier
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.
going to revert your commit and do the fixes directly so I can merge asap
FYI I edited the whole tutorial on this PR to make it more coherent, and cleaned up some mistakes. Thanks for working on this, going to merge now |
Thanks @slorber |
Motivation
Attempt to fix #4079, with small consistent commits. This pr is a follow up to #4302.
This is my progress so far
RoadMap
/cc @ShinteiMai
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)