update Contributors directive to merge to main
@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.
@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 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 https://github.com/invoke-ai/InvokeAI/pull/1984#issuecomment-1351330298, 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
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 main had been updated since this PR was submitted I had to do an "Update from main" operation before I could do the merge. This of course triggered the CI tests, meaning I would have to come back to this PR in about half an hour to do the actual merge.
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. 😂