Skip to content

Conversation

@pkuczynski
Copy link
Collaborator

@pkuczynski pkuczynski commented Nov 11, 2025

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

  • Code is up-to-date with the master branch
  • This pull request links relevant issues as Fixes #00000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change

Summary by CodeRabbit

  • Chores
    • Updated documentation search indexing workflow to improve build reliability and prevent unnecessary processing on forked repositories.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 11, 2025

Walkthrough

The 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

Cohort / File(s) Summary
GitHub Actions Workflow Configuration
.github/workflows/docsearch.yml
Job name standardized from index_docs to index-docs. Added fork prevention guard (if: ${{ !github.event.repository.fork }}). Removed explicit step name for checkout action. Expanded docker run environment variables with TYPESENSE_PORT="443", TYPESENSE_PROTOCOL="https", and CONFIG containing JSON configuration content.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Verify fork prevention condition is correct for intended behavior
  • Confirm Typesense environment variable values match intended deployment configuration
  • Validate JSON CONFIG format is properly escaped in YAML

Suggested reviewers

  • gioboa

Poem

🐰 The workflow hops with purpose true,
Forks are blocked, clean steps shine through,
Typesense spreads its wings so wide,
With HTTPS and port, it glides with pride! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: preventing the docs index CI workflow from running on forks, which matches the core objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between ba3319d and 981df0e.

📒 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 from index_docs to index-docs is 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 tostring command 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 input

The original concern about unescaped characters breaking the environment variable assignment is not applicable here—the code is safe as written.

@coveralls
Copy link

Coverage Status

coverage: 76.433%. remained the same
when pulling 981df0e on pkuczynski:ci/no-index-on-forks
into ba3319d on typeorm:master.

Copy link
Collaborator

@gioboa gioboa left a comment

Choose a reason for hiding this comment

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

Great 👏

@pkuczynski pkuczynski merged commit e04ffd3 into typeorm:master Nov 12, 2025
10 checks passed
@pkuczynski pkuczynski deleted the ci/no-index-on-forks branch November 12, 2025 09:52
ThbltLmr pushed a commit to ThbltLmr/typeorm that referenced this pull request Dec 2, 2025
mgohin pushed a commit to mgohin/typeorm that referenced this pull request Jan 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants