Skip to content

Add workflow to automatically label pull requests#9121

Closed
hashhar wants to merge 1 commit intotrinodb:masterfrom
hashhar:hashhar/auto-label
Closed

Add workflow to automatically label pull requests#9121
hashhar wants to merge 1 commit intotrinodb:masterfrom
hashhar:hashhar/auto-label

Conversation

@hashhar
Copy link
Copy Markdown
Member

@hashhar hashhar commented Sep 5, 2021

For starting with this will add the docs label to any changes under the docs tree.

We can eventually add more rules.
The end-goal is to use these labels to selectively run tests or run additional tests.

e.g. for docs the product tests don't make sense to run. Also for example if there are changes in Hive related code the tests:hive label could be added automatically to ensure all the Hive product test configs run.

In this PR no actions are being taken based on the label so nothing changes except triaging might be a little easier now.

@Praveen2112
Copy link
Copy Markdown
Member

Can we have any test PRs which is labeled as doc based on this workflow ?

name: "Pull Request Labeler"

on:
pull_request_target:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the value of the "pull_request_target" => ? association here?

guessing it it an empty map, consider:

Suggested change
pull_request_target:
pull_request_target: {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, shouldn't we limit this to types: [open]? let's allow humans to unlabel something, for example if only docs/READE is changed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything in the docs tree is docs related .. even if it is just the readme or the pom or whatever .. but yes, we should also allow unlabelling

@@ -0,0 +1,18 @@
name: "Pull Request Labeler"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just labeler should be fine (see ci.yml)

- uses: actions/labeler@v3
with:
repo-token: "${{ secrets.GITHUB_TOKEN }}"
sync-labels: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we go with types:open, we should remove this as no longer applicable

with:
repo-token: "${{ secrets.GITHUB_TOKEN }}"
sync-labels: true
configuration-path: .github/config/labeler-config.yml
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any particular reason not to use the default location (.github/labeler.yml)?

Copy link
Copy Markdown
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am good with the idea and approach. Would be interested to figure out if this also updates the label if files change? E.g. a code PR does not have the label but then an update to some docs is added in a follow up push. Imho this should then add the docs label.. we can however merge and revise later.

Also I hope this does not add too much time to the build overall.

name: "Pull Request Labeler"

on:
pull_request_target:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anything in the docs tree is docs related .. even if it is just the readme or the pom or whatever .. but yes, we should also allow unlabelling

@hashhar
Copy link
Copy Markdown
Member Author

hashhar commented Dec 10, 2021

This was done in a different way in #9674

That has the benefit that it'll work correctly even if someone manually mislabels or files change after the PR is created.

@hashhar hashhar closed this Dec 10, 2021
@hashhar hashhar deleted the hashhar/auto-label branch December 10, 2021 08:12
@findepi
Copy link
Copy Markdown
Member

findepi commented Dec 10, 2021

@hashhar auto-adding docs label is still valuable eg for human consumption, even if what-to-test decision is determined differently (#9674)

@hashhar
Copy link
Copy Markdown
Member Author

hashhar commented Dec 10, 2021

Fair point, I'll create a new one/re-open after verifying if it works even when changes are made to an already open PR.

@findepi
Copy link
Copy Markdown
Member

findepi commented Dec 13, 2021

after verifying if it works even when changes are made to an already open PR.

Do you think it's a must-have? it's not a must-have for me.

FWIW see labelling in https://github.com/apache/iceberg project: https://github.com/apache/iceberg/pulls/

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Jan 18, 2022

e.g. for docs the product tests don't make sense to run.

I think we should add docs label when the PR contains docs change and it doesn't contain other changes if we want to use it for tests filter.

@hashhar
Copy link
Copy Markdown
Member Author

hashhar commented Jan 18, 2022

I think we should add docs label when the PR contains docs change and it doesn't contain other changes

Should be possible by changing existing label config to:

docs:
  - all: ['docs/**']

if we want to use it for tests filter.

I think using labels for tests filtering has the downside that labels are applied after CI run has already been dispatched.
The way you implemented the docs check seems more robust.

IMO labels are for human consumption/informational. Test filtering should be as automatic as possible (like it is for docs-check).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants