search

narwhals-dev / narwhals

Lightweight and extensible compatibility layer between dataframe libraries!
https://narwhals-dev.github.io/narwhals/
MIT License
572 stars 89 forks source link

Fix autolabeler #490

Closed DeaMariaLeon closed 2 months ago

DeaMariaLeon commented 4 months ago

The release-drafter only looks for lowercase titles on new PRs. But even when the PRs have the right format, it's not working.

Needs investigation.

DeaMariaLeon commented 4 months ago

It looks like something changed for the GITHUB_TOKEN (like a permit or something). This error seems like the action wasn't allowed to do its thing: https://github.com/narwhals-dev/narwhals/actions/runs/9898914239

MarcoGorelli commented 4 months ago

hey - it looks like it worked here https://github.com/narwhals-dev/narwhals/pull/496, and it a few others?

MarcoGorelli commented 4 months ago

I think it needs to be in the right format to begin with, and that once someone's opened a PR, correcting the title later doesn't change labels?

FBruzzesi commented 4 months ago

A label is automatically generated if as Marco said the title has the "right" format to start with. I wonder what happens if we add the label afterwards manually - if the drafter is based on that I image it would still pick it up on merge?!

DeaMariaLeon commented 4 months ago

There is no action on merge for the autolabeler I think. Maybe we should add it. Also the regex could be improved. I could experiment on my toy project.

FBruzzesi commented 3 months ago

Should we consider this done, or do we want to go the extra mile with regex? 👀

DeaMariaLeon commented 3 months ago

I can add a bit of regex. At least to accept when the word starts with uppercase. As in ^([B|b]uild) for example. WDYT?

FBruzzesi commented 2 months ago

I think it would lead to some less work from our side when a PR is opened/merged, but I don't want to end up discouraging people from contributing because the title doesn't pass a regex check 🤌🏼😂

DeaMariaLeon commented 2 months ago

Regex is already being used on the drafter, but it only checks for lowercase. If the regex expression finds something, it labels the PR correctly.

If we add the uppercase check, the drafter will label the PRs more often (starting with "Build" or with "build", for example). Nothing blocks, or will block, the contributor to open a PR with the wrong title.

FBruzzesi commented 2 months ago

Loud and clear! Thanks a lot for explaining the mechanism 😁 I definitly support that

DeaMariaLeon commented 2 months ago

Working on this now