Skip to content
This repository has been archived by the owner on Jul 1, 2024. It is now read-only.

Allow owners to do a "ready to merge" self-merge #156

Merged
merged 1 commit into from
Jul 21, 2020
Merged

Allow owners to do a "ready to merge" self-merge #156

merged 1 commit into from
Jul 21, 2020

Conversation

elibarzilay
Copy link
Contributor

Add text to the comment telling owners that it can be done --- except
when there are too many owners, or when there are multiple packages.
(Owners can still do it if there are many, only multiple packages block
the whole thing.)

Closes #98.

Also remove an unused const approvals line.

Add text to the comment telling owners that it can be done --- *except*
when there are too many owners, or when there are multiple packages.
(Owners can still do it if there are many, only multiple packages block
the whole thing.)

Closes #98.

Also remove an unused `const approvals` line.
@orta
Copy link
Contributor

orta commented Jul 20, 2020

Cool, yeah - the code looks right to me, but I'm a bit unsure about why we get a lot more new comments in the tests when the fixtures didn't get updated

@elibarzilay
Copy link
Contributor Author

There's a bunch of ignorable changes since there is one un-conditional new \n in the string now. Could be removed with

and I'll merge this PR almost instantly. Thanks for helping out! :heart:${
otherOwners.length === 0 ? "" : `\n
(${otherOwners.map(o => "@" + o).join(", ")}: you can do this too.)`}`});

But I suspected that it's too much. (I'm fine doing this change it if you think it's worth it.)

Copy link
Contributor

@orta orta left a comment

Choose a reason for hiding this comment

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

👍 - yeah, probably that the SHA changed in the body and so it didn't get detected by the duplication lookup

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

There's no way to declare that module is auto generated
2 participants