Set PR Roadmap from target branch when merged - #59
Conversation
Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
There was a problem hiding this comment.
Pull request overview
This PR replaces the previous actions/github-script implementation of the add-to-project composite action with a Python-based implementation that can optionally derive and set the GitHub Project “Roadmap” field for PRs in NVIDIA/cudf-spark and NVIDIA/cudf-spark-jni by reading the target branch root pom.xml at the immutable base SHA.
Changes:
- Replaced the composite action step from
actions/github-script@v8to a Python runner invokingadd_to_project.py. - Added
add_to_project.pyto (1) add issues/PRs to the configured GitHub Project and (2) auto-populate “Roadmap” from the target branchpom.xmlfor the specified repositories, only when the field is currently empty.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| add-to-project/add_to_project.py | New standard-library GitHub REST/GraphQL client and Roadmap auto-population logic based on target-branch pom.xml. |
| add-to-project/action.yml | Switches the composite action implementation from github-script to running the new Python script with the provided token. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Greptile SummaryThe PR replaces the JavaScript-based composite-action implementation with a Python GitHub API client that preserves project membership and derives merged pull requests’ Roadmap values from the target branch POM.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[GitHub event] --> B[Load issue or pull request]
B --> C[Find configured Project V2]
C --> D{Item already in project?}
D -- No --> E[Add project item]
D -- Yes --> F[Reuse existing item]
E --> G{Merged PR in supported repository?}
F --> G
G -- No --> H[Finish]
G -- Yes --> I{Roadmap already set?}
I -- Yes --> H
I -- No --> J[Read root POM at merge-result SHA]
J --> K[Derive YY.MM and resolve option]
K --> L[Refresh Roadmap]
L --> M{Still empty?}
M -- No --> H
M -- Yes --> N[Set Roadmap option]
N --> H
Reviews (6): Last reviewed commit: "Keep Roadmap fix minimal" | Re-trigger Greptile |
| return base_ref, base_sha, pom_xml | ||
|
|
||
|
|
||
| def populate_roadmap(client, repository, pull_request, project, item): |
There was a problem hiding this comment.
The function body never references pull_request, can it be removed?
Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
|
One more concern: the “add to project” workflow runs only on GitHub’s This is currently sufficient for spark-rapids, because project membership does not need to change between release cycles. However, if a PR remains open from 26.10 until 27.10, the Roadmap should be determined when the PR is merged. In that case, it should be set to 27.10, not 26.10. Ideally, the Roadmap should be set by the GitHub Actions workflow when the PR is merged. |
Thanks, that makes sense. I propose keeping opened only for adding the PR to the Project, then setting Roadmap on closed only when the PR was merged. |
Make sense |
Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
Updated and tested in my personal repo for both |
## Summary - keep adding issues and pull requests to the NVIDIA Project when they are opened - also run the shared action when a pull request is closed and merged - skip pull requests that are closed without merging - use `spark-rapids-common/add-to-project@main` ## Behavior After NVIDIA/spark-rapids-common#59 is merged, the shared action adds the PR to the Project on `opened`, but only derives and sets `Roadmap` on the merged `closed` event. If `Roadmap` was assigned manually before merge, it is preserved. ## Testing note This is a draft integration PR and depends on NVIDIA/spark-rapids-common#59. `pull_request_target` uses the workflow from the base repository's default branch, so this PR's new `closed` trigger will not execute until the workflow change exists on the default branch. ## Validation - YAML parsed successfully - `git diff --check` - event-condition review: issue opened and PR opened run; merged PR runs; closed-unmerged PR is skipped Common action PR: NVIDIA/spark-rapids-common#59 --------- Signed-off-by: YanxuanLiu <yanxuanl@nvidia.com>
Summary
NVIDIA/cudf-sparkandNVIDIA/cudf-spark-jni, read the rootpom.xmlat the event's immutable merged-result SHA and derive theYY.MMreleaseRoadmapfield only when it is empty, preserving existing values and re-checking immediately before the updateWhy
Roadmap should reflect the release branch at merge time. A pull request that stays open across release cycles is therefore assigned from the branch state it actually merges into, rather than from its state when opened.
Consumer workflow updates add the merged
closedevent while continuing to add issues and pull requests onopened:Validation
git diff --check