Skip to content

[workflow] Trigger format server when @taichi-gardener is requested for review#1026

Merged
archibate merged 9 commits intotaichi-dev:masterfrom
archibate:cmhook
May 22, 2020
Merged

[workflow] Trigger format server when @taichi-gardener is requested for review#1026
archibate merged 9 commits intotaichi-dev:masterfrom
archibate:cmhook

Conversation

@archibate
Copy link
Collaborator

@archibate archibate commented May 20, 2020

@archibate archibate changed the title [skip ci] add comment.yml to trigger jobs on comment [workflow] Add comment.yml to trigger jobs on comment May 20, 2020
@archibate archibate requested review from k-ye and yuanming-hu May 20, 2020 06:56
@archibate
Copy link
Collaborator Author

Hello? Feel free to say No if you don't like this...

@yuanming-hu
Copy link
Member

Hello? Feel free to say No if you don't like this...

Sorry, I've got some work to do over the day, but I'll take a look at this later. Thanks for the PR!

@yuanming-hu
Copy link
Member

My biggest worry about this is that, if a PR author adds a comment for formatting their code, all the reviewers will receive that comment as well. This creates too much noise in the reviewers' mailbox. As a result, they will start to ignore emails from GitHub regarding Taichi.

@archibate
Copy link
Collaborator Author

Then, how about:
Only hook on edited, not created.
So that we can edit the PR description to trigger /format.

@archibate
Copy link
Collaborator Author

I just come up with that, we can hook on review_requested event:
When @taichi-gardener was requested for review, the format server is triggered.

@yuanming-hu
Copy link
Member

I just come up with that, we can hook on review_requested event:
When @taichi-gardener was requested for review, the format server is triggered.

This sounds great!!

@archibate archibate changed the title [workflow] Add comment.yml to trigger jobs on comment [workflow] Trigger format server when @taichi-gardener is requested fo review May 22, 2020
@archibate archibate changed the title [workflow] Trigger format server when @taichi-gardener is requested fo review [workflow] Trigger format server when @taichi-gardener is requested for review May 22, 2020
@archibate archibate requested a review from taichi-gardener May 22, 2020 03:08
@archibate archibate removed the request for review from taichi-gardener May 22, 2020 03:14
@archibate
Copy link
Collaborator Author

archibate commented May 22, 2020

OK now!
During @taichi-gardener is on the list of reviewers, all pushes to the PR will trigger a format.
When @taichi-gardener is requested for review, immediately trigger a format at once.

Copy link
Member

@yuanming-hu yuanming-hu left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM! Just one minor place that needs discussions.

Comment on lines -6 to -7

[[Click here for the format server]](http://kun.csail.mit.edu:31415/)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should keep this here for a while, just in case some newcomers do not have access to reviewer assignment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, sorry, I almost forget that review requesting needs write-access...

@archibate
Copy link
Collaborator Author

We will make persubmit.yml to do the work at some point (after I found a way to write repo in gha).

@archibate archibate requested a review from yuanming-hu May 22, 2020 15:28
@archibate
Copy link
Collaborator Author

Hello? I want to merge this for 'enjoyment' in other prs.

Copy link
Member

@yuanming-hu yuanming-hu left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you! Let's see how it works :-)

@archibate archibate merged commit 8248673 into taichi-dev:master May 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants