-
-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Prevent PRs outside of TryGhost org from attempting browser tests #20067
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
base: main
Are you sure you want to change the base?
Conversation
31b972f
to
6ac6c6d
Compare
Our bot has automatically marked this PR as stale because there has not been any activity here in some time. If we’ve missed reviewing your PR & you’re still interested in working on it, please let us know. Otherwise this PR will be closed shortly, but can always be reopened later. Thank you for understanding 🙂 |
WalkthroughThe changes in this update affect two areas: the GitHub Actions workflow configuration and a source file. In the ✨ Finishing Touches
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. 🪧 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 (
|
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
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
121-124
: Output variable for fork detection added
The newis_fork
output correctly identifies PRs from forked repos using the GitHub context. Ensure this string output aligns with subsequent comparisons (e.g., comparing against'true'
). Consider renaming tois_forked_pr
for clearer intent.
290-296
: Prevent browser tests on forked PRs
The updatedif
condition forjob_browser-tests
addsneeds.job_get_metadata.outputs.is_fork != 'true'
to skip tests on forked PRs, which satisfies the PR objective. Double‑check that the string comparison aligns with the output format. Optionally, use== 'false'
for better readability.
return app; | ||
// REMOVE THIS | ||
}; |
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.
Remove placeholder comment
The // REMOVE THIS
line appears to be a leftover placeholder and does not serve any purpose in production code. Please remove it to keep the codebase clean.
no issue