-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add the PR commented state #40
Conversation
da26dbc
to
1791f72
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks !!
Could you test the changes before merging? :)
@@ -60,10 +64,14 @@ def main(config: Config) -> None: | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chouetz
Since completely removing :review_started: could be a possible breaking change (no high risk though, it's just slack emojis.
What we can possibly do is to not add "review started" when "commented" is defined.
from:
new_emojis = {config.emoji_review_started}
into:
new_emojis = {}
if not config.emoji_commented:
new_emojis.add(config.emoji_review_started)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edit: wrong PR to comment on for the test, sorry about that 😬
793d949
1791f72
to
793d949
Compare
793d949
to
6685870
Compare
6685870
to
443d699
Compare
not tested