-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
ci: prevent running docs index on forks #11762
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
Conversation
WalkthroughThe documentation search indexing workflow was updated to prevent execution on forked repositories, simplified step naming for the checkout action, and expanded Typesense configuration with additional environment variables for protocol, port, and JSON configuration settings. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 0
🧹 Nitpick comments (1)
.github/workflows/docsearch.yml (1)
14-14: Consider restoring the step name for improved clarity.Removing the step name (
name: Checkout code) makes the workflow output slightly less readable. While unnamed steps are valid, named steps improve CI log visibility and debugging. Consider adding a descriptive name back.- - uses: actions/checkout@v5 + - name: Checkout repository + uses: actions/checkout@v5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/docsearch.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: tests-linux (20) / better-sqlite3
- GitHub Check: tests-linux (20) / mysql_mariadb_latest
- GitHub Check: tests-linux (20) / cockroachdb
- GitHub Check: tests-linux (20) / sqljs
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-linux (18) / sap
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-linux (18) / postgres (17)
- GitHub Check: tests-linux (18) / postgres (14)
- GitHub Check: tests-linux (18) / mongodb
- GitHub Check: tests-linux (18) / sqlite
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-windows / sqljs
- GitHub Check: tests-windows / sqlite
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
.github/workflows/docsearch.yml (3)
10-10: Correct and idiomatic use of fork detection to prevent unnecessary failures.The conditional
if: ${{ !github.event.repository.fork }}is the standard GitHub Actions way to skip jobs on forks. This directly addresses the PR objective—forked repositories lack repository secrets and the workflow would fail without this guard.
9-9: Job name change fromindex_docstoindex-docsis safe and follows GitHub Actions conventions.Verification confirms no references to the old job name exist in the repository, workflows, or documentation. The kebab-case naming aligns with GitHub Actions best practices and will not break any external dependencies.
20-22: No issues found. The JSON escaping is handled correctly.The
jq -r tostringcommand properly converts the JSON config to a single-line string with all special characters safely escaped. Verification confirms:
- ✓ No unescaped shell metacharacters ($, `, or internal newlines)
- ✓ The converted string remains a single line (988 characters)
- ✓ Docker
-e CONFIG="..."syntax receives properly formatted inputThe original concern about unescaped characters breaking the environment variable assignment is not applicable here—the code is safe as written.
gioboa
left a comment
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.
Great 👏
Description of change
It does not make sense to run this workflow when forks are synced, since they won't have secrets and will fail anyway.
Pull-Request Checklist
masterbranchFixes #00000Summary by CodeRabbit