rematocorp / trello-integration-action

GitHub action for connecting GitHub PRs and Trello cards — moves cards, adds labels & people, and more
MIT License
17 stars 9 forks source link

Addressing review #112

Closed Crembotz closed 2 days ago

Crembotz commented 3 days ago

Hi. I was wondering if it would be possible to add the functionality to move Trello tasks to different columns based on the latest submitted review, for example: If I submitted a review for a PR and I requested for changes to be made then I'd like to move the card to a certain list and if I submitted a review for a PR and I approved it then I'd like to move it to a different list.

Thanks!

ukupat commented 3 days ago

Hi! Yes, I think this is possible. What if there are multiple reviews, for example: one approves and the other requests changes. Do I assume correctly that then the action should move the Trello task to the "Changes required" list?

Here is the full list of review statuses https://docs.github.com/en/graphql/reference/enums#pullrequestreviewstate. Could you please outline me how do you want to manage your Trello tasks based on different situations? I'm more than happy to implement it.

Crembotz commented 3 days ago

Regarding your question: yeah in that case the card would move to a Pending for fixes list. I'll note that we're a small team and up until now each PR had one reviewer assigned to it. So the way we manage tasks is as follows: We create a new PR and the card is moved to a list called pending for a review. A team member goes through the PR and submits it with the request changes option, then we would move the card to a list called pending for fixes - in this case the relevant value from the enum would be REQUEST_CHANGES After the person assigned to the task addressed the comments made, we would move back the card to pending for a review list. This would go on until the PR is approved, in this case the card would be moved to the Approved list. After the branch was merged, we would move the card to the Done list. Hope this clears things up.

ukupat commented 3 days ago

Thank you! One more question:

After the person assigned to the task addressed the comments made

Does this mean that the person clicks on the "Re-request review" button? Or something else?

Screenshot 2024-07-04 at 08 03 28
Crembotz commented 2 days ago

I'll admit that up until now we've just notified each other via a group chat but your suggestion is the better alternative and the approach we'll take from now on :)

ukupat commented 2 days ago

Hehe cool, we use this little button too :) I'll try to implement your feature request soon. I'll let you know when you can test it out!

Crembotz commented 2 days ago

Thank you so much!

ukupat commented 2 days ago

@Crembotz aaalright, please test v9.5.0, it includes these changes https://github.com/rematocorp/trello-integration-action/pull/113/files. Let me know if you need help or if something is not working correctly :)

Crembotz commented 2 days ago

You are incredible, you've done it really really fast! I'll try it out tomorrow and let you know of my findings, thank you so much!

Crembotz commented 1 day ago

@ukupat I've ran my tests and everything exactly as I expected, thank you so much for getting this done, this will be a huge help! One note about the readme file: in the sections of the fields trello-list-id-pr-changes-requested and trello-list-id-pr-approved you said that you need to add the review_requested and review_request_removed types to pull_request_review trigger. After checking the documentation, the types associated with this trigger are submitted, edited and dismissed so you should update the readme file. Thanks again for your help!

ukupat commented 1 day ago

I actually meant this

Screenshot 2024-07-05 at 12 44 41

I will try to improve the readme, thanks for the feedback :) I'm glad it works. Let me know if your team finds some anomalies :)

Crembotz commented 1 day ago

So pull_request_review is supposed to stay empty?

ukupat commented 1 day ago

I believe

pull_request_review:

equals

pull_request_review:
    types: [submitted, edited, dismissed]

so it is up to you if you want to write the types out or not :)