search

unblu / ucascade

Working with multiple main branches on GitLab
https://unblu.github.io/ucascade
Apache License 2.0
17 stars 7 forks source link

Add original description on cascading MR #28

Closed kakawait closed 6 months ago

kakawait commented 6 months ago

Fixes #27

Contains several things:

  1. Cascading MR description
  2. Add more style / emoji on cascade MR description
  3. fix (possible?) bug on GitLab CE (on our side MR was not assigned when conflict, I was suspecting was related that GitLab CE only accept 1 assignee, multiple is for EE/Paid version)

Tests passed locally without changing it. I've tried to do not increase the number of GitLab API calls!

--

PS: Sorry some emergency not yet find to time to cleanup the git log. I'll try asap but I still propose in case of

jmini commented 6 months ago

Thank you for the PR. We will have a look ASAP.

Sorry some emergency not yet find to time to cleanup the git log.

our policy is to squash the commits of a PR, so it doesn't matter for the history.

jmini commented 6 months ago

fix (possible?) bug on GitLab CE (on our side MR was not assigned when conflict, I was suspecting was related that GitLab CE only accept 1 assignee, multiple is for EE/Paid version)

This is something where a config might be interesting. We have the commercial version of GitLab where we can assign multiple assignee to one MR.

But when testing ucascade on gitlab.com where we only have a free account, we noticed that only one assignee was possible. In that case it would make sense to keep only the assignee of the original MR.

And for users of the commercial version, keep it as it is today: add all assignees of the original MR + the merge-user to the ucascade MR.

kakawait commented 6 months ago

fix (possible?) bug on GitLab CE (on our side MR was not assigned when conflict, I was suspecting was related that GitLab CE only accept 1 assignee, multiple is for EE/Paid version)

This is something where a config might be interesting. We have the commercial version of GitLab where we can assign multiple assignee to one MR.

But when testing ucascade on gitlab.com where we only have a free account, we noticed that only one assignee was possible. In that case it would make sense to keep only the assignee of the original MR.

And for users of the commercial version, keep it as it is today: add all assignees of the original MR + the merge-user to the ucascade MR.

What I've did is:

If we're using GitLab CE, btw the number of assignee can't be more than 1 (so if (cascadeResponsibleIds.size() > 1) always false), thus I'm using withAssigneeId and even if you're on EE edition if you've assignee only 1 there is no reason that withAssigneeId can't work. So I didn't need to check CE or EE version :)

fbpe commented 6 months ago

If we're using GitLab CE, btw the number of assignee can't be more than 1 (so if (cascadeResponsibleIds.size() > 1) always false)

Even using Gitlab CE, the cascadeResponsibleIds.size() can be > 1 at this point in the code. If the user that merged the original MR was not its assignee, we consider both the MR merger and the MR assignee as cascade responsibles. This is where we process this logic: https://github.com/unblu/ucascade/blob/a216a79183fa54b66856eaf9aeff9d0f73e5e89e/src/main/java/service/GitLabService.java#L551-L553

Nonetheless, I also agree that we can add the configuration to distinguish between Gitlab CE/EE in a follow-up Issue/PR.

For the scope of this PR, it looks good to me like this.