NuKeeperDotNet / NuKeeper

Automagically update nuget packages in .NET projects
Apache License 2.0
540 stars 128 forks source link

Add relevant reviewers to PRs to local BitBucket repositories instead of everyone in the Project #1108

Closed arikalish closed 2 years ago

arikalish commented 3 years ago

:sparkles: What kind of change does this PR introduce? (Bug fix, feature, docs update...)

Bug fix? I guess?

:arrow_heading_down: What is the current behavior?

When creating a PR to a locally-hosted BitBucket repository everyone set as a default reviewer in the Project, including users that no longer exist, are added as reviewers.

This results in people who don't need to see the PRs being added to them erroneously.

Also, default reviewer rules are not respected, so folks that should be put on PRs are not added.

If a user no longer exists NuKeeper fails due to the presence of an error section in the response, even though BitBucket also provides a list of users.

For reference, repositories live inside of projects in BB, so we might have:

Project X

If Project X has reviewers: A: All branches B: Branches named "foo-" C: Branches named "bar-" Repo: D: "foo-*"

Example cases:

PR of branch "abc" into Repo: Users A, B, and C will be added to the PR but user D will not be. Expected behavior in that case would be for user A to be added and no one else.

PR of branch "foo-123" into Repo: Users A, B, and C will be added to the PR but user D will not be. Expected behavior: Users A, B, and D are added.

PR of branch "bar-456" into Repo: Users A, B, and C will be added to the PR but user D will not be. Expected behavior: Users A and B are added.

:new: What is the new behavior (if this is a feature change)?

Call the default reviewers API on the BitBucket instance to get the list of expected reviewers from BitBucket itself and filter out any inactive (aka: deleted) users that might have been left in rules on the server. BitBucket will apply the rules that would be applied if a user had created the PR manually.

:boom: Does this PR introduce a breaking change?

Yes, but the existing behavior was likely not what was intended

:bug: Recommendations for testing

See above example cases, also test with users that have been removed from BitBucket but are still present in default reviewer rules.

:memo: Links to relevant issues/docs

N/A

:thinking: Checklist before submitting

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

arikalish commented 3 years ago

Still relevant

skolima commented 2 years ago

Looks good. Thanks.