Skip to content

CI perm and LFS - decide on solving messy merge

We encountered a messy situation with merging:

What did happen:

  1. I started working on: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5735,
  2. I later got information from Patricio that he is working on: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6043,
  3. I based my changes from !5735 (closed) (MR for CI permissions) on LFS branch, because we were working on similar changes, and incorporated most of Patricio improvements,
  4. Today when doing final round of testing !5735 (closed) (MR for CI permissions) we discovered that LFS changes are not fully covered with tests: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6043#note_15689584,
  5. I decided to create !6409 (merged) (MR for CI permissions without LFS) that is based on !6043 (merged) (LFS), but reverts LFS related changes: https://gitlab.com/gitlab-org/gitlab-ce/commit/6d43c95b7011ec7ec4600e00bdc8df76bb39813c,
  6. I asked Jacob to do final review of !6409 (merged) (MR for CI permissions without LFS). However, Remy clicked merge before Jacob did finish,
  7. !6409 (merged) (MR for CI permissions without LFS) got merged,
  8. Because !6409 (merged) (MR for CI permissions without LFS) were incorporating changes from !6043 (merged) (LFS) and reverting part of them by this commit https://gitlab.com/gitlab-org/gitlab-ce/commit/6d43c95b7011ec7ec4600e00bdc8df76bb39813c it did also close !6043 (merged) (LFS).

The biggest problem right now is that:

  1. !6409 (merged) is merged, and Jacob did not finish the final review of it,
  2. !6043 (merged) is marked as merged, but in fact it's not part of master, because of https://gitlab.com/gitlab-org/gitlab-ce/commit/6d43c95b7011ec7ec4600e00bdc8df76bb39813c

Because of the !6409 (merged) it become in a little messy situation.

We are looking for opinions what to do now. Possible options:

  1. Revert !6409 (merged) (MR for CI permissions without LFS) and recreate MR for !6043 (merged) (LFS) and !6409 (merged),
  2. Leave !6409 (merged), finish final review and create a new MR for !6043 (merged) with https://gitlab.com/gitlab-org/gitlab-ce/commit/6d43c95b7011ec7ec4600e00bdc8df76bb39813c being reverted,

cc @stanhu @DouweM @jacobvosmaer-gitlab @patricio @rymai

Edited by 🤖 GitLab Bot 🤖