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

Ignore codeowners that got previously removed from review #72007

Closed

Conversation

Gamer025
Copy link
Contributor

@Gamer025 Gamer025 commented Dec 14, 2022

About The Pull Request

Only merge this after the following 2 PRs got merged!
tgstation/RequestReviewFromUser#5
tgstation/RequestReviewFromUser#6

Makes it so that codeowners no longer get notified if they got removed from review previously in the PRs lifetime
Requested by @Mothblocks

Why It's Good For The Game

Apparently no means no and the action constantly re-adding you as owner after you remove yourself is annoying

Changelog

@tgstation-server tgstation-server added the GitHub Remember that time they had to get us to send them a copy of the repo label Dec 14, 2022
@Mothblocks
Copy link
Member

Why is this an opt in configuration? I can't imagine anyone ever wanting this off

@Gamer025
Copy link
Contributor Author

Gamer025 commented Dec 15, 2022

Why is this an opt in configuration? I can't imagine anyone ever wanting this off

Because if I made it a default I would have needed to increase the version number to 2 because it would be a breaking change, which would have also meant needing to make a V2 branch, because of how Github actions works
Also nobody else ever complained about it so it seems to maybe be preferred by some/most?

@Mothblocks
Copy link
Member

Mothblocks commented Dec 16, 2022

I think it's a bug fix more than a breaking change tbh

I don't think it's preferred I think I'm just the first person to remove themselves from reviews

@Mothblocks Mothblocks removed the request for review from dragomagol December 18, 2022 21:27
@Wayland-Smithy
Copy link
Contributor

Wayland-Smithy commented Dec 21, 2022

needing to make a V2 branch

Neat. I didn't know you could do that with branches. I have always gone the tag/release route or to be extra safe used the exact commit hash.

@Gamer025
Copy link
Contributor Author

needing to make a V2 branch

Neat. I didn't know you could do that with branches. I have always gone the tag/release route or to be extra safe used the exact commit hash.

Yeah I got the idea from actions/toolkit#214
While the official docs recommend just moving the V1 tag I really don't like the idea of moving already published tags

@Mothblocks
Copy link
Member

I'm gonna close this since we agreed (I think) on coderbus that this should be considered a bug fix and require no config changes

@Mothblocks Mothblocks closed this Dec 21, 2022
@Mothblocks Mothblocks reopened this Dec 21, 2022
@@ -27,3 +27,4 @@ jobs:
with:
separator: ' '
users: ${{ steps.CodeOwnersParser.outputs.owners }}
ignoreRemovedUsers: true
Copy link
Member

Choose a reason for hiding this comment

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

gamer025 wants me to test something

@Gamer025 Gamer025 requested review from Mothblocks and removed request for dragomagol December 21, 2022 23:58
@Mothblocks Mothblocks closed this Dec 21, 2022
@Gamer025 Gamer025 deleted the ignored-removed-codeowners branch April 17, 2023 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GitHub Remember that time they had to get us to send them a copy of the repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants