Skip to content
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

Disabling member assigning completely #114

Closed
Crembotz opened this issue Jul 14, 2024 · 27 comments · Fixed by #116
Closed

Disabling member assigning completely #114

Crembotz opened this issue Jul 14, 2024 · 27 comments · Fixed by #116

Comments

@Crembotz
Copy link

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
Copy link
Member

ukupat commented Jul 14, 2024

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
Copy link
Author

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

@ukupat
Copy link
Member

ukupat commented Jul 14, 2024

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

@Crembotz
Copy link
Author

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

@ukupat
Copy link
Member

ukupat commented Jul 15, 2024

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
Copy link
Author

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
Copy link
Member

ukupat commented Jul 15, 2024

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
Copy link
Author

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
Copy link
Member

ukupat commented Jul 16, 2024

@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
Copy link
Author

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
Copy link
Author

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

@Crembotz
Copy link
Author

@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 ukupat reopened this Jul 20, 2024
@ukupat
Copy link
Member

ukupat commented Jul 20, 2024

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
Copy link
Member

ukupat commented Jul 21, 2024

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
Copy link
Author

Crembotz commented Jul 21, 2024

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
Copy link
Author

Crembotz commented Jul 23, 2024

@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
Copy link
Member

ukupat commented Jul 24, 2024

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
Copy link
Author

  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
Copy link
Member

ukupat commented Jul 24, 2024

  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
Copy link
Author

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
Copy link
Member

ukupat commented Jul 24, 2024

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

@Crembotz
Copy link
Author

Crembotz commented Jul 25, 2024

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
Copy link
Member

ukupat commented Jul 25, 2024

Hmm can you share the logs again?

@Crembotz
Copy link
Author

@ukupat
Copy link
Member

ukupat commented Jul 26, 2024

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

@Crembotz
Copy link
Author

Crembotz commented Jul 26, 2024

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
Copy link
Member

ukupat commented Jul 26, 2024

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

@ukupat ukupat closed this as completed Jul 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants