-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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: add non-English PR flagging workflow #12934
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Should only match markdown and JSON files, excluding all English JSONs.
with: | ||
github-token: ${{secrets.GITHUB_TOKEN}} | ||
script: | | ||
const prNumber = context.issue.number; |
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.
(GitHub treats PR's as a type of issue)
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.
context
made available from the Octokit package, which is automatically made available with actions/github-script@v5
on: | ||
pull_request: | ||
paths: | ||
- "public/content/translations/**/*.md" | ||
- "src/intl/**/*.json" | ||
- "!src/intl/en/** |
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.
Will check every PR for changes to these paths. If any file in the diff includes:
- a markdown file within
public/content/translations
or - a JSON file within
src/intl
(but NOT insrc/intl/en
)
the job will run.
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.
Only .md
and .json
; images and other files could be updated separately without Crowdin.
- name: Exit early if branch name contains 'crowdin' | ||
run: | | ||
if [[ "${{ github.head_ref }}" == *crowdin* ]]; then | ||
echo "Branch name contains 'crowdin', stopping workflow" | ||
exit 1 | ||
fi |
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.
Exits if crowdin
is found in the branch name. The utils.ts
update in this PR includes the addition of this string segment to the branch names generated in the Crowdin CI flow, so these will not be flagged.
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.
Exiting with 1
(a non-zero status) will be seen as a "failure" and cause the following job to not run.
fi | ||
|
||
add_label_and_comment: | ||
needs: check_branch_name |
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.
Only if the branch is not a crowdin
branch will this run
Placing back in draft while I investigate a way to ignore |
Challenging to find a clean approach to identifying that only href values were changed. Instead, patched with an update to identify if the changed lines at least include the string segment This still needs some testing; keeping in draft in the meantime
|
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.
Looks great, two nitpicks but your call. LGTM
const commentWithoutHrefs = `This pull request contains changes to non-English content, which must be handled through the Crowdin platform instead of GitHub.` | ||
const commentWithHrefs = `This pull request contains changes to non-English content files, which may need to be handled through the Crowdin platform instead of GitHub. |
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.
We don't want people to see this comment and close their PR, what do you think of adding this?
const commentWithoutHrefs = `This pull request contains changes to non-English content, which must be handled through the Crowdin platform instead of GitHub.` | |
const commentWithHrefs = `This pull request contains changes to non-English content files, which may need to be handled through the Crowdin platform instead of GitHub. | |
const commentWithoutHrefs = `This pull request contains changes to non-English content, which must also be handled through the Crowdin platform instead of GitHub.` | |
const commentWithHrefs = `This pull request contains changes to non-English content files, which may also need to be handled through the Crowdin platform instead of GitHub. |
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.
Sure! Just would also change "instead of GitHub." to "instead of only on GitHub." (or something similar)
@coderabbitai review |
Actions PerformedReview triggered.
|
WalkthroughThe recent updates introduce a GitHub Actions workflow to monitor non-English content updates and enforce the use of Crowdin for translations. Additionally, a minor adjustment was made to the branch naming convention in the locale translation pull request function, adding a "crowdin-" prefix for clearer identification. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 2
Co-authored-by: Joshua <62268199+minimalsm@users.noreply.github.com>
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.
LGTM
Description
crowdin
; this allows us to easily ignore those branches)Summary by CodeRabbit
New Features
Improvements