Skip to content
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

Jump to bottom

Take "needs" labels seriously, block merge if labels are assigned #48359

Merged
merged 4 commits into from
Jan 20, 2023
Merged

Take "needs" labels seriously, block merge if labels are assigned #48359

merged 4 commits into from
Jan 20, 2023

Conversation

IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Jan 20, 2023 •
edited
Loading

This is a proposal to ensure that needs docs or needs tests etc. labels are effective before PRs with them are merged and go out of sight into the merged PRs list.

Also does the same for the DO NOT MERGE label.

The idea being that this check would be a "required" check.

@IanButterworth IanButterworth requested a review from a team as a code owner January 20, 2023 03:25
DilumAluthge
DilumAluthge requested changes Jan 20, 2023
View reviewed changes
Copy link
Member

@DilumAluthge DilumAluthge left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  1. Can you add a short timeout? E.g. 2 minutes or 3 minutes.
  2. Can you add a permissions key that gives only read permissions to the GITHUB_TOKEN?

.github/workflows/LabelCheck.yml Outdated Show resolved Hide resolved
@DilumAluthge DilumAluthge added the ci Continuous integration label Jan 20, 2023
@DilumAluthge
Copy link
Member

Ohhh, now I remember why we need synchronize - to make sure the LabelCheck status always shows up in the web UI. @IanButterworth Can you add it back?

@DilumAluthge
Copy link
Member

Let's test that it actually works.

@DilumAluthge DilumAluthge added DO NOT MERGE Do not merge this PR! needs more info Clarification or a reproducible example is required and removed DO NOT MERGE Do not merge this PR! needs more info Clarification or a reproducible example is required labels Jan 20, 2023
DilumAluthge
DilumAluthge requested changes Jan 20, 2023
View reviewed changes
.github/workflows/LabelCheck.yml Outdated Show resolved Hide resolved
@IanButterworth IanButterworth changed the title RFC: Take "needs" labels seriously, block merge if labels are assigned Take "needs" labels seriously, block merge if labels are assigned Jan 20, 2023
@DilumAluthge
Copy link
Member

Let's give this a try and see how it goes. Easy enough to revert if we don't like it.

@DilumAluthge DilumAluthge merged commit 1481a89 into JuliaLang:master Jan 20, 2023
@IanButterworth IanButterworth deleted the ib/label_check branch January 20, 2023 17:47
@lgoettgens lgoettgens mentioned this pull request Oct 13, 2023
Merged
@LilithHafner LilithHafner mentioned this pull request Oct 14, 2023
Merged
3 tasks
@visr visr mentioned this pull request Jul 6, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DilumAluthge DilumAluthge DilumAluthge approved these changes

Assignees
No one assigned
Labels
ci Continuous integration
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@IanButterworth @DilumAluthge