-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Added workflow to auto-tag docs PRs based on distro sync PRs. #5545
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
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
a6341d9
to
6775b43
Compare
6775b43
to
ba5efc4
Compare
a3ce724
to
5224870
Compare
.github/workflows/autotag_docs.yml
Outdated
run: | | ||
|
||
# Testing permissions of github token (must be removed before merge) | ||
gh pr edit 762 --repo ros-navigation/docs.nav2.org --add-label documentation |
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.
Should this still be here?
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.
I added it as a method to show you that the secrets.GITHUB_TOKEN
does not have enough permissions to label PRs in another repository within the same workspace.
I think the permissions for this secret need to be updated.
I have dropped it now for the review.
.github/workflows/autotag_docs.yml
Outdated
# Determine ROS distro and label from PR title | ||
pr_title="${{ github.event.pull_request.title }}" | ||
|
||
if [[ "$pr_title" == *"Kilted Sync"* ]]; then |
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.
Is this because I name backport PRs as "XYZ Sync"?
Something to keep in mind is that I don't manually backport everything. Sometimes people backport themselves manually or are cherry picked using the backport-XYZ
existing mergify job. If we only tagged PRs based on the PR title for my bulk backports, we'd miss alot
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.
Maybe instead we should more generally just look at PRs that are targeting specific branches instead (i.e. the branch to be merged into is humble/jazzy/kilted). Then we can have the branch name in the matrix to run over and apply the label to respective docs PRs. That would then catch my backport syncs (independent of the PR name) and any other backports manually done by someone else or mergify.
In other words, we can use a build matrix workflow, whereas the matrix is set by branch name (which makes it very easy to add / remove them in the future). This job is triggered by a PR open against only one of those branches (humble, jazzy, etc) to attempt to look internally to that PR to find the doc PRs.
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.
Thanks for the suggestion, Steve.
Yes, my initial thought was that all backports were titled as Distro Sync
, but this is more accurate.
I have updated the workflow to run on specific targeted branches instead - kilted
, jazzy
, and humble
.
echo "Labeling docs PR #$docs_pr with '$label'" | ||
gh pr edit $docs_pr --repo ros-navigation/docs.nav2.org --add-label "$label" | ||
else | ||
echo "No cross-referenced docs PR found for navigation2 PR #$pr" |
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.
If we know a PR is a backport and we don't find a docs PR, I think that's a legit reason to fail. We can then handle the failures on a case-by-case (i.e. no docs needed), but that way we know something was missed
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.
I added a conditional that now fails, if the entire backport collection of PRs do not have any docs PRs associated with them at all.
Like auto-opening a PR against that distro branch? Sounds nice! |
I think we should also add a docs PR to our PR template so that they have a natural place to be set to link https://github.com/ros-navigation/navigation2/blob/main/.github/PULL_REQUEST_TEMPLATE.md and say that all PRs adding features, adding or changing parameters, or changing defaults need to have doc PRs. |
5224870
to
23dffb2
Compare
Yes, indeed it does :) |
I have updated the PR template to include a |
Signed-off-by: Leander Stephen D'Souza <[email protected]>
Signed-off-by: Leander Stephen D'Souza <[email protected]>
Signed-off-by: Leander Stephen D'Souza <[email protected]>
90f77bc
to
af9cd96
Compare
Note
Backport labels need to be created on ros-navigation/docs.nav2.org prior in order for tagging.
Basic Info
Description of contribution in a few bullet points
humble
,jazzy
, andkilted
.#1234
in the title.documentation
field.Description of documentation updates required from your changes
Description of how this change was tested
The embedded logic can be tested out in the following bash script. Make sure log in using
gh auth login
if not configured before:Expected Output:
By default, the bash script targets Kilted Sync Sept 19, 2025 1.4.2 #5539; this can be passed as an argument as follows:
Note that in this example, the label would be empty. However, the linked docs PR would show up.
Since I do not have permissions to label, I think that once the labels are created in the Docs repository, this can be tested by the maintainers to verify its working.
Future work that may be required in bullet points
For Maintainers:
backport-*
.