Skip to content

Allow refactor label to meet CI requirement #2573

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.

Sign up for GitHub

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

Merged
amomchilov merged 2 commits into from
Apr 2, 2026
Merged

Allow refactor label to meet CI requirement #2573

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5a3ea0a
Allow `refactor` label to meet requirement
amomchilov Apr 1, 2026
9b7619d
Improve CI failure message
amomchilov Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/release.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ changelog:
labels:
- dependencies
- chore
- refactor
authors:
- dependabot
categories:
Expand Down
9 changes: 8 additions & 1 deletion bin/check_pr_labels
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ REQUIRED_LABELS = [
"bugfix",
"chore",
"enhancement",
"refactor",
Copy link
Copy Markdown
Member

@paracycle paracycle Apr 1, 2026

Choose a reason for hiding this comment

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

This on its own isn't enough, we need to also classify the tag to feed into one of the release categories here: https://github.com/Shopify/tapioca/blob/main/.github/release.yml#L10-L23

Actually, maybe, instead of manually maintaining this list, we should just check against the union of labels for all the categories

Copy link
Copy Markdown
Contributor

@Morriar Morriar Apr 1, 2026

Choose a reason for hiding this comment

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

We could continue using chore for this as it doesn't bring much value for end users

Copy link
Copy Markdown
Member

@paracycle paracycle Apr 1, 2026

Choose a reason for hiding this comment

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

I can also see us adding refactor to the exclude list.

Copy link
Copy Markdown
Contributor Author

@amomchilov amomchilov Apr 1, 2026

Choose a reason for hiding this comment

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

I like adding it to the exclude list.

"ignore-for-release",
"security",
"sorbet",
Expand All @@ -26,7 +27,13 @@ labels = JSON.parse(arg)["labels"].map { |label| label["name"] }
matching_labels = labels & REQUIRED_LABELS

if matching_labels.empty?
warn("PR is missing at least one of the following labels: #{REQUIRED_LABELS.join(", ")}")
warn <<~MSG
Copy link
Copy Markdown
Contributor Author

@amomchilov amomchilov Apr 1, 2026

Choose a reason for hiding this comment

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

This error will always be triggered on community contributed PRs, whose authors wouldn't have the access level necessary to label it themselves.

We talked about this with the team a while ago, and didn't have a good solution. For now, let's at least give more clear instruction.

Copy link
Copy Markdown
Contributor Author

@amomchilov amomchilov Apr 1, 2026

Choose a reason for hiding this comment

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

cc @ufuk @Morriar

Tapioca has an automated release process which generates the release notes based on PRs labels.
(See https://github.com/Shopify/tapioca/blob/main/.github/release.yml)

Please ask a Shopify employee to label your PR with at least one of the following labels:
#{REQUIRED_LABELS.map { |label| " - #{label}" }.join("\n")}
MSG
exit(1)
end

Expand Down
Loading