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.

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 by Bors] - Relicense bors checks #2493

Closed
wants to merge 2 commits into from

Conversation

DJMcNab
Copy link
Member

@DJMcNab DJMcNab commented Jul 17, 2021

Objective

Currently we can sometimes allow PRs by people who haven't agreed to the relicense to get merged into main.

E.g. #2445

Solution

This adds a check to ensure that this doesn't happen, by ensuring that bors doesn't relicense said PRs.

As a bonus, it also adds config to automatically label new PRs as needing checking, to ensure that we do the verification until we merge the new license.

@mockersf
Copy link
Member

Not a fan of the auto label but it makes sense and shouldn't be for long

@DJMcNab
Copy link
Member Author

DJMcNab commented Jul 17, 2021

Ok, here's a weird thing - this PR hasn't been automatically labelled with S-Needs-Triage. That seems strange

@DJMcNab
Copy link
Member Author

DJMcNab commented Jul 17, 2021

Yes, indeed. The automatic labelling is only temporary. I'd rather make sure that we don't have any false approvals again, so it's best to check.

@DJMcNab DJMcNab added A-Meta About the project itself C-Feature A new feature, making something new possible labels Jul 17, 2021
@mockersf
Copy link
Member

Ok, here's a weird thing - this PR hasn't been automatically labelled with S-Needs-Triage. That seems strange

That shouldn't come from the change in this PR as the label workflow is pull_request_target so it's using config file from main... still strange

@DJMcNab
Copy link
Member Author

DJMcNab commented Jul 17, 2021

Ok, this is 'expected' - actions/labeler doesn't match files/folders starting with a . from a glob pattern (because it doesn't have https://github.com/isaacs/minimatch#dot set)

Copy link
Member

@NiklasEi NiklasEi left a comment

Choose a reason for hiding this comment

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

Definitely better to remove a few premature labels than to merge a PR and then have to run after the contributor to approve the license change 👍

@cart
Copy link
Member

cart commented Jul 17, 2021

bors r+

bors bot pushed a commit that referenced this pull request Jul 17, 2021
# Objective

Currently we can sometimes allow PRs by people who haven't agreed to the relicense to get merged into main.

E.g. #2445

## Solution

This adds a check to ensure that this doesn't happen, by ensuring that bors doesn't relicense said PRs.

As a bonus, it also adds config to automatically label new PRs as needing checking, to ensure that we do the verification until we merge the new license.
@bors bors bot changed the title Relicense bors checks [Merged by Bors] - Relicense bors checks Jul 17, 2021
@bors bors bot closed this Jul 17, 2021
@DJMcNab DJMcNab deleted the relicense-checks branch July 17, 2021 19:24
ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
# Objective

Currently we can sometimes allow PRs by people who haven't agreed to the relicense to get merged into main.

E.g. bevyengine#2445

## Solution

This adds a check to ensure that this doesn't happen, by ensuring that bors doesn't relicense said PRs.

As a bonus, it also adds config to automatically label new PRs as needing checking, to ensure that we do the verification until we merge the new license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Meta About the project itself C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants