update Contributors directive to merge to main #1984
Conversation
The answer is for the reviewing team to be careful before approving a PR and not do it unless they're absolutely sure it is good to go. @lstein I wonder if it is possible to set 2 mandatory approval requirements for new or non approved contributors and 1 for the rest? |
|
I don't think that there's a way to require more approvals for one type of
contributor than another.
The auto-merge feature is an option, and I suggest that we only use it for
very small fixes. Reviewers are able to set and unset it as well, and I
have been setting it to off for anything I have any uncertainty about.
Happy to discuss.
Lincoln
…On Wed, Dec 14, 2022 at 8:11 AM blessedcoolant ***@***.***> wrote:
@lstein <https://github.com/lstein> - Assuming we're having people merge
to main (still calling out doing that AND having an auto-merge is high
risk), updating directive to do so.
The answer is for the reviewing team to be careful before approving a PR
and not do it unless they're absolutely sure it is good to go.
@lstein <https://github.com/lstein> I wonder if it is possible to set 2
mandatory approval requirements for new or non approved contributors and 1
for the rest?
—
Reply to this email directly, view it on GitHub
<#1984 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAA3EVP2NFC65ZMKIQEQXJ3WNHBRHANCNFSM6AAAAAAS55ONMI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
*Lincoln Stein*
Head, Adaptive Oncology, OICR
Senior Principal Investigator, OICR
Professor, Department of Molecular Genetics, University of Toronto
Tel: 416-673-8514
Cell: 416-817-8240
***@***.***
*E**xecutive Assistant*
Michelle Xin
Tel: 647-260-7927
***@***.*** ***@***.***>*
*Ontario Institute for Cancer Research*
MaRS Centre, 661 University Avenue, Suite 510, Toronto, Ontario, Canada M5G
0A3
@OICR_news
<https://can01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftwitter.com%2Foicr_news&data=04%7C01%7CMichelle.Xin%40oicr.on.ca%7C9fa8636ff38b4a60ff5a08d926dd2113%7C9df949f8a6eb419d9caa1f8c83db674f%7C0%7C0%7C637583553462287559%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=PS9KzggzFoecbbt%2BZQyhkWkQo9D0hHiiujsbP7Idv4s%3D&reserved=0>
| www.oicr.on.ca
*Collaborate. Translate. Change lives.*
This message and any attachments may contain confidential and/or privileged
information for the sole use of the intended recipient. Any review or
distribution by anyone other than the person for whom it was originally
intended is strictly prohibited. If you have received this message in
error, please contact the sender and delete all copies. Opinions,
conclusions or other information contained in this message may not be that
of the organization.
|
|
I think its okay to stay. We can be a tad bit careful and I think it'll have its advantages in the long run. So we don't sit around waiting for checks to pass on PR's that we have fully approved and cleared. |
|
Case in point on this PR. I reviewed and approved it, and all checks had passed, but because Rather than having to come back, I then set auto-merge to true. So now if the CI tests pass (which they should!), the merge will happen without this additional work. |
|
And meanwhile, despite automerge being enabled its... stuck in an "out-of-date" with base branch status. 😂 |
hipsterusername
force-pushed
the
contributor-readme-update
branch
from
d010813
to
06f7f9f
Compare
@lstein - Assuming we're having people merge to main (still calling out doing that AND having an auto-merge is high risk), updating directive to do so.