-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: update roomotes.yml to checkout PR branch before running translation check #6539
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,8 +1,31 @@ | ||||||
| version: "1.0" | ||||||
|
|
||||||
| commands: | ||||||
| - name: Pull latest changes | ||||||
| run: git pull | ||||||
| - name: Fetch and checkout PR branch | ||||||
| run: | | ||||||
| # Try to get PR info from various possible environment variables | ||||||
| if [ -n "$GITHUB_HEAD_REF" ]; then | ||||||
| echo "Checking out PR branch: $GITHUB_HEAD_REF" | ||||||
| git fetch origin "$GITHUB_HEAD_REF" | ||||||
| git checkout -B "$GITHUB_HEAD_REF" "origin/$GITHUB_HEAD_REF" | ||||||
| elif [ -n "$GITHUB_REF" ] && [[ "$GITHUB_REF" =~ ^refs/pull/([0-9]+)/ ]]; then | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this intentional to use bash-specific syntax without explicitly requiring bash? The
This could cause the script to fail silently in non-bash environments. |
||||||
| PR_NUMBER="${BASH_REMATCH[1]}" | ||||||
| echo "Fetching PR #$PR_NUMBER" | ||||||
| git fetch origin "pull/$PR_NUMBER/head:pr-$PR_NUMBER" | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same error handling concern here - these git commands could fail silently. Adding error checks would make failures more visible. |
||||||
| git checkout "pr-$PR_NUMBER" | ||||||
| elif [ -n "$GITHUB_EVENT_PATH" ] && [ -f "$GITHUB_EVENT_PATH" ]; then | ||||||
| # Try to extract PR number from GitHub event JSON | ||||||
| PR_NUMBER=$(jq -r '.pull_request.number // .number // empty' "$GITHUB_EVENT_PATH" 2>/dev/null) | ||||||
| if [ -n "$PR_NUMBER" ]; then | ||||||
| echo "Fetching PR #$PR_NUMBER from event data" | ||||||
| git fetch origin "pull/$PR_NUMBER/head:pr-$PR_NUMBER" | ||||||
| git checkout "pr-$PR_NUMBER" | ||||||
| else | ||||||
| echo "Could not determine PR number from event data" | ||||||
| fi | ||||||
| else | ||||||
| echo "No PR information found in environment, using current branch" | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we make this fallback message more informative? It would be helpful to know which branch is actually being used:
Suggested change
|
||||||
| fi | ||||||
| timeout: 60 | ||||||
| execution_phase: task_run | ||||||
| - name: Install dependencies | ||||||
|
|
||||||
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.
Could we add error handling here? If the git fetch or checkout commands fail (network issues, permission problems, etc.), the script will continue and potentially run the translation check on the wrong branch. Consider: