-
Notifications
You must be signed in to change notification settings - Fork 130
Closing / Fixing policy #76
Comments
@snostorm et al.: should we delete branches after merge, or leave that to the PR submitter? |
Meh. I sometimes do occasional cleanup of stale branches off repos while drinking my morning coffee. Zero preference on the timing. (Part of me likes the idea of a few hour buffer for some odd reason.) |
I also think we should have a 2 or more 👍 from WG on a PR before merging. or 2 or more 👎 before closing without merge. |
2 seems reasonable. |
Also, what about someone who wants to "dibs" a merge for one reason or another? Should we allow for that, and check who the assignee is before merging, or should we do first-come-first-serve? |
Bump |
Even core only requires a single reviewer. |
Core only requires a single reviewer but that assumes they are the expert in that particular area and it specifies that they let the PR sit for more than a day to give people a chance to weigh in. We're merging much faster so it might be worth it to change the policy to require 2 or more +1 but remove the requirement on letting PR's sit for so long. |
+1 @mikeal |
Someone should write up whatever we agree to and stick it in the CONTRIBUTING file ;) |
It seems the x day policy is still undefined. (First WG meeting minutes, search #76, third WG meeting minutes, search #76.) Are we going to leave this undefined for now, add it to the next WG-meeting agenda, or pick a time frame? @indexzero suggested 72 hours here. |
I say we pick a timeframe and stick with it. +1 to 72 hours with no activity. |
Would this apply in all PRs that is commented on with a question or request, or just ones that have merge conflicts? |
Sent in PR #232. |
Further discussion taking place in #232, I will close this issue for now. |
We need a policy for closing/fixing PRs that have merge conflicts. see #24-comment-70226567. @snostorm suggested we close after some sort of X day policy. First maybe tag the author to try and rebase/conflict resolve before closing?
The text was updated successfully, but these errors were encountered: