-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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: use dumi to build new docs #5378
Conversation
WalkthroughThe pull request introduces modifications to the project's configuration and build process. Changes include updating the Vercel production deployment workflow to target a different branch, adding the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@@ -5,7 +5,7 @@ env: | |||
on: | |||
push: | |||
branches: | |||
- master | |||
- master-skip-releases |
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.
还需要时间去重构,先不构建
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected] |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/vercel-production.yml
(1 hunks).gitignore
(1 hunks)package.json
(2 hunks)site/config/config.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- site/config/config.ts
- .gitignore
🔇 Additional comments (2)
package.json (2)
72-72
: LGTM! Good choice of documentation tool.
The addition of dumi v2.4.17 is a good choice for building modern documentation:
- It has good TypeScript support
- It's actively maintained
- It's well-suited for React-based documentation
103-104
: LGTM! Scripts simplified appropriately.
The script changes are appropriate:
- Removed Node 14-16 specific scripts as they're no longer needed (project requires Node.js >= 18.19.0)
- Removed unnecessary
NODE_OPTIONS=--openssl-legacy-provider
flag which was likely needed for older Node.js versions
Let's verify the dumi scripts work with the current Node.js version:
✅ Verification successful
Let me verify the dumi dependency and configuration in the project.
Let me check the site configuration and dumi setup.
The dumi scripts are properly configured but require installation
The verification shows that:
- Dumi is correctly configured as a dev dependency (version ^2.4.17)
- There's a valid dumi configuration file at
site/config/config.ts
- The
npx dumi
command failed because node_modules are not available in the sandbox environment
The scripts are correctly defined and will work in the actual development environment where:
- Dependencies are installed (
npm install
oryarn install
) - Node.js >= 18.19.0 is used (as per project requirements)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if dumi commands are recognized and show their help output
npx dumi -h 2>/dev/null || echo "Dumi command not found"
Length of output: 68
Script:
#!/bin/bash
# Check package.json for dumi dependency and version
cat package.json | jq '.dependencies.dumi, .devDependencies.dumi'
# Check if there's a dumi config file
fd -e ts -e js "\.dumi\.(ts|js)$|config\.(ts|js)$" --type f
Length of output: 678
Script:
#!/bin/bash
# Check site config content
cat site/config/config.ts
# Check if there are any dumi-specific files in the site directory
fd . site/ -t f | grep -i dumi
# Check if there's a README or documentation about site setup
cat site/README.md 2>/dev/null || echo "No site README found"
Length of output: 2618
@@ -5,7 +5,7 @@ env: | |||
on: | |||
push: | |||
branches: | |||
- master | |||
- master-skip-releases |
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.
💡 Codebase verification
Production deployment workflow should use master
branch instead of non-existent master-skip-releases
The verification reveals several issues:
- The
master-skip-releases
branch does not exist in the repository master
is the default branch and is used by other critical workflows:- CI workflow runs on
master
- GitHub Pages deployment runs on
master
- The preview deployment explicitly ignores
master
(as it should)
- CI workflow runs on
The change to use master-skip-releases
appears to be incorrect because:
- The branch doesn't exist, which would break the production deployment
- The repository uses
master
as its main branch for all other workflows - There's a separate
skip-releases
branch used only for the release workflow, suggestingmaster-skip-releases
was mistakenly combined
Recommendation:
- Revert the branch configuration back to
master
in the Vercel production deployment workflow
🔗 Analysis chain
Verify the production deployment branch change
The branch for production deployment has been changed to master-skip-releases
. This seems unusual for a production workflow. Could you please clarify:
- Is this intentional as part of the dumi migration?
- Why use a branch with "skip-releases" in its name for production deployment?
- Will this affect the documentation deployment process?
Let's check the branch setup:
Let me check the repository structure and workflow configurations to better understand the deployment setup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the target branch exists and its recent activity
git branch -r | grep master-skip-releases
git log origin/master-skip-releases -n 1 2>/dev/null || echo "Branch not found"
Length of output: 43
Script:
#!/bin/bash
# Check all workflow files and their branch configurations
echo "=== Checking workflow files ==="
fd -e yml -e yaml . .github/workflows/ --exec cat {}
echo -e "\n=== Checking default branch ==="
git symbolic-ref refs/remotes/origin/HEAD || echo "Default branch reference not found"
echo -e "\n=== Checking active branches ==="
git branch -r
Length of output: 4483
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5378 +/- ##
=======================================
Coverage 88.36% 88.36%
=======================================
Files 39 39
Lines 3772 3772
Branches 338 338
=======================================
Hits 3333 3333
Misses 439 439 ☔ View full report in Codecov by Sentry. |
Summary by CodeRabbit
dumi
as a development dependency.gitignore
to exclude.dumi
directory in the site folder