This repository has been archived by the owner on Aug 16, 2023. It is now read-only.
[MIRROR] Use draft pull requests instead of the 'Work In Progress' and 'Needs Review' labels. #581
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…Review' labels. (#56168) Conflicts: tools/WebhookProcessor/github_webhook_processor.php
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Original PR: tgstation/tgstation#56168
Why
Draft PRs are a system maintained by Github that we've basically duplicated with the
Work in Progress/Needs Reviewlabels and auto-tagger. Any maintainer can convert a PR to a draft and vice-versa.It makes a lot more sense to lose these labels and use the draft system. Here's a few reasons why:
Changes Requestedstatus. This helps maintainers to see what PRs need review at a glance. The problem with that is when the contributor addresses the review, it sticks around. I attempted to get around this limitation in the past by crafting an auto-review dismissal system if all review comments were resolved, but this had it's own setbacks. Instead, I propose maintainers, after reviewing a PR, set it to be a draft and contributors can change it back to ready once they've addressed issues.[WIP]and[READY]in this case). This is already a heavy-handed method (and makes PR titles ugly as sin) and is completely unnecessary with the draft system where it's just a click of a button.TL;DR: If a PR is green, it's either ready to merge and needs to be looked at by a maintainer, waiting for 24h, or should be test merged. Otherwise, should be grey.
Upon this being merged, delete the two labels.
Side note: This PR also fixes a bug in the auto commenting system that's supposed to remind people to write changelogs which I'm pretty sure we don't use anyway.
@ tgstation/commit-access