-
Notifications
You must be signed in to change notification settings - Fork 735
Badges to PR body #29702
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?
Badges to PR body #29702
Conversation
|
🟢 |
|
Backported to |
This commit updates the `validate_pr_description.py` script to check for the presence of a legend in the PR body before appending it. If the legend already exists, it will not be added again, improving the clarity and conciseness of PR descriptions. Key changes: - Added a check for the legend's existence in the PR body. - Modified the logic for appending tables and the legend to ensure no duplicates are included.
This commit enhances the `test_validation.py` script by adding functionality to automatically determine the GitHub repository from the git remote URL. This improvement allows for better local testing by setting the `GITHUB_REPOSITORY` environment variable if it is not already defined. Key changes: - Introduced `find_github_repository` function to extract the repository name from the git remote URL. - Updated the main function to set `GITHUB_REPOSITORY` for local testing, improving usability and reducing manual setup requirements. - Enhanced documentation to clarify the automatic setting of `GITHUB_WORKSPACE` and `GITHUB_REPOSITORY` environment variables.
…te_pr_description.py`. The script now directly uses the PR body for validation, streamlining the process and improving usability.
This commit modifies the `validate_pr_description` workflow to introduce two new environment variables: `SHOW_RUN_TESTS_IN_PR` and `SHOW_BACKPORT_IN_PR`. These flags control the inclusion of test execution and backport tables in the PR body. The `ensure_tables_in_pr_body` function is updated to handle these flags, allowing for more flexible table generation based on user requirements. Key changes: - Replaced `SHOW_ADDITIONAL_INFO_IN_PR` with the new flags in the action configuration. - Updated `test_validation.py` to reflect the new environment variables and their usage. - Enhanced `validate_pr_description.py` to conditionally add tables based on the new flags, improving the clarity and usability of PR descriptions.
This commit enhances the `generate_backport_table` function in `validate_pr_description.py` to collect and sort unique branches from the input. The changes ensure that the backporting URL is generated for all unique branches, improving the accuracy and clarity of the backport table in PR descriptions. Key changes: - Updated logic to collect unique branches from multiple entries. - Sorted branches for consistent output. - Modified the backport button label for clarity.
|
⚪
🟢 |
|
⚪
🟢 |
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.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Try to get PR number from remaining args | ||
| if len(sys.argv) > idx + 2: | ||
| try: | ||
| pr_number = int(sys.argv[idx + 2]) | ||
| except ValueError: | ||
| pass |
Copilot
AI
Nov 28, 2025
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.
The argument parsing logic has an issue. When --body-file is used with a PR number (e.g., test_validation.py --body-file file.txt 12345), the PR number would be at index 3, but the code tries to parse it from sys.argv[idx + 2] where idx is the index of --body-file (index 1). This means sys.argv[3] would be the PR number, which is correct. However, if the command is test_validation.py 12345 --body-file file.txt, then idx is 2, and sys.argv[4] would be accessed, which might not exist. The logic should handle different argument orders more robustly.
| url = f"{base_url}?{query_string}" | ||
| url_ui = f"{base_url}?{query_string}&ui=true" | ||
|
|
||
| rows.append(f"| [}-4caf50?style=flat-square)]({url}) []({url_ui}) |") |
Copilot
AI
Nov 28, 2025
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.
The badge label for backport branches at line 192 uses branch.replace('-', '_') but doesn't handle commas. When a branch entry contains multiple branches separated by commas (e.g., "stable-25-2,stable-25-2-1,stable-25-3"), the commas will remain in the badge label, which may cause issues with URL encoding or badge rendering. Consider also replacing commas with underscores or another separator: branch.replace('-', '_').replace(',', '_').
| rows.append(f"| [}-4caf50?style=flat-square)]({url}) []({url_ui}) |") | |
| rows.append(f"| [.replace(',', '_')}-4caf50?style=flat-square)]({url}) []({url_ui}) |") |
| if not run_id: | ||
| raise ValueError("GITHUB_RUN_ID environment variable is not set") | ||
|
|
||
| return f"{github_server}/{github_repo}/actions/runs/{run_id}" |
Copilot
AI
Nov 28, 2025
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.
The PR comment logic identifies comments by workflow run URL (line 35), but the "Update PR comment with results" step runs within the matrix job (lines 245-256 in run_tests.yml). Since all matrix jobs share the same GITHUB_RUN_ID, they will all try to update the same comment. This will cause the last completing matrix job to overwrite the results from other jobs, losing information about tests for other branches. Consider either:
- Creating separate comments per matrix branch by including the branch in the identifier
- Moving the update step to a separate job that runs after all matrix jobs complete and aggregates results
- Including the matrix branch context in the comment identification and creating one comment per branch
This commit updates the issue pattern matching in `pr_template.py` to use a negative lookahead, preventing false matches with branch names in URLs. Additionally, the `validate_pr_description.py` script is modified to check for the "Bugfix" category in a case-insensitive manner, improving the accuracy of PR description validation. Key changes: - Updated issue pattern to avoid matching branch names. - Added case-insensitive check for "Bugfix" category in PR descriptions.
|
⚪
🟢 |
Co-authored-by: Copilot <[email protected]>
|
⚪
🟢 |
|
⚪
🟢 |
|
|
Backported to |
Что добавлено
Критерии приёмки
Changelog entry
...
Not for changelog (changelog entry is not required)
Changelog category
Description for reviewers
...
🔄 Backport
Legend: