rematocorp / trello-integration-action

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

Disabling member assigning completely #114

Closed Crembotz closed 1 month ago

Crembotz commented 1 month ago

Hello! I've noticed that the automation assigns the person that reviewed the PR to the Trello card and I was wondering if it would be possible to completely disable this, we have our own way of handing assigning people to tasks and we don't Github to interfere. Thank you!

ukupat commented 1 month ago

Hi! Hmm do you assign PRs to code reviewers (https://docs.github.com/en/issues/tracking-your-work-with-issues/assigning-issues-and-pull-requests-to-other-github-users)? I ask this because the action shouldn't assign reviewers (https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/requesting-a-pull-request-review) to Trello card. It should only assign PR author, all PR commits authors and PR assignees.

Crembotz commented 1 month ago

Okay you've made the situation clearer, can I disable the assignment of the PR author?

ukupat commented 1 month ago

Yes, to confirm: you want noone to be assigned to the Trello card based on the PR info?

Crembotz commented 1 month ago

Precisely. We handle assigning people to the Trello cards by ourselves so we don't any info from the PR

ukupat commented 1 month ago

Cool, I will try to make it happen today-tomorrow. If it is not a secret, can you share what logic do you use to assign people the Trello cards? Just curious :)

Crembotz commented 1 month ago

Of course, nothing too sophisticated. We're currently only two programmers on the team so let's say I create a PR for a task, so the other guy will be my reviewer. After the PR was created and the card was moved to a Pending for review list, I get unassigned and he gets assigned. After he's done and he either requests for changes or approves the PR , he gets unassigned and I get reassigned. This goes on until the PR is approved and merged.

ukupat commented 1 month ago

Hmm yeah, this flow is familiar to me. Maybe you want me to implement this flow into the action (with some extra boolean conf)?

Crembotz commented 1 month ago

That's what I had in mind: that you would have a flag that will control whether people get assigned to a card or not

ukupat commented 1 month ago

@Crembotz ciao! Please check out v9.6.0. It now includes two new flags: trello-add-members-to-cards and trello-switch-members-in-review. The first one disables this whole members management system so you could do it manually the way you like. The second flag tries to simulate how you guys work. I hope at least one of them works for you :)

Crembotz commented 1 month ago

Thank you so much for addressing my issue! I'll try to run a small test and will let you know of my findings.

Crembotz commented 1 month ago

I'll have to postpone the tests, I'll try it out as soon as I can and will let you know

Crembotz commented 1 month ago

@ukupat Hi, so we've ran our tests and got some inconsistent results: when moving a card from the pending for review list to the pending for fixes list, the assigned member wouldn't get updated. When the PR was created and the card was moved to the pending for review list, the author of the PR got unassigned and the reviewer got assigned and then for some reason got unassigned.

ukupat commented 1 month ago

Thank you! Weird results. I don't have any good ideas why is this happening. I will create a PR tomorrow with some extra logging so you could run both of the cases again and forward the logs to me. I hope that this is okay.

ukupat commented 1 month ago

Can you retest the first problem (when moving a card from the pending for review list to the pending for fixes list, the assigned member wouldn't get updated) with this version:

- uses: rematocorp/trello-integration-action@trello-switch-members-in-review-vol-2

and share the logs with me (feel free to obfuscate data you don't feel like sharing).

About the second issue (when the PR was created and the card was moved to the pending for review list, the author of the PR got unassigned and the reviewer got assigned and then for some reason got unassigned), can you confirm that you don't have any custom Trello Butler automation that removes/changes members when the card is moved?

Crembotz commented 1 month ago

I'll have the first test's results by Tuesday. We have Butler automations but we disabled them. I'll run more tests to make sure they indeed didn't have anything to do with the final results we got.

Crembotz commented 1 month ago

@ukupat Here are the logs, notice that for each action(i.e: moving from pending for review to pending for fixes) I've also specified the behavior that occurred in the file's name, hope this helps! https://drive.google.com/file/d/1lk4bfzlRg8PVCW0ZFc5EP9M27DTfQaFu/view?usp=sharing

ukupat commented 1 month ago

Thanks! Can you confirm a few things:

  1. From the logs I can see that the liranpeleg5 requested changes. Is this correct?
  2. From the logs I can see that the action assigns the card to you (Trello username eransimhon). Are you the author of the PR?
  3. If 2. is correct, were you actually assigned to the card?
  4. From the logs I can see that trello-remove-unrelated-members is false. If this is turned off then the action does not remove previously assigned code reviewer (liranpeleg5). I didn't think of it before... 🤔 Do you need to keep it turned off?
Crembotz commented 1 month ago
  1. Yes
  2. Yes
  3. When the card was moved from pending for review to either pending for fixes or approved, it did assign me but it didn't remove the reviewer(liranpeleg5).
  4. I can turn it on, do you think it will yield the desired behavior?
ukupat commented 1 month ago
  1. Ahaa, I understand now.
  2. No need anymore. I made sure that reviewer is removed without enabling trello-remove-unrelated-members. Could you give it another go?

You can also test the 2nd issue if the 1st issue is solved:

When the PR was created and the card was moved to the pending for review list, the author of the PR got unassigned and the reviewer got assigned and then for some reason got unassigned.

Crembotz commented 1 month ago

Thank you, should I still use this - uses: rematocorp/trello-integration-action@trello-switch-members-in-review-vol-2 or a different version? I'll be able to test this tomorrow

ukupat commented 1 month ago

Yes, continue using - uses: rematocorp/trello-integration-action@trello-switch-members-in-review-vol-2 :)

Crembotz commented 1 month ago

So the problem persists: if a card is moved from pending for review to either pending for fixes or approved, it assigns back the PR author but doesn't remove the reviewer. I didn't change any flag values.

ukupat commented 1 month ago

Hmm can you share the logs again?

Crembotz commented 1 month ago

https://drive.google.com/file/d/1pGmtP8r1ryg3NZcBhZy9Oo1jCyrdp9IN/view?usp=sharing Hope this helps

ukupat commented 1 month ago

Yes, the logs helped! I found my silly bug 🐛 Give it another go 🤞

Crembotz commented 1 month ago

You fixed it! It works exactly as intended and will make our workflow so much easier to manage! Thank you so much for your help! Please let me know after you've merged your fixes and what version I should use.

ukupat commented 1 month ago

Great! I have made the release, please use - uses: rematocorp/trello-integration-action@v9.6.1