clean up accidential merge of !16
When merging the simple documentation fix !70 (merged) I accidentially merged !16 (merged) too. This issue is for coming up with any bright idea how to clean this up.
Activity
Title changed from clean up accidential merge of #16 (closed) to clean up accidential merge of !16 (merged)
I see the following options:
- reset master on the server. That will probably mess up a lot of peoples repositories and possibly internal gitlab state, so not really an option.
- revert the changes. That will mess up the repositories of those people that are using the branch, in particular if parts of the branch are to be merged later for real. I think the affected people would be @joe, @christi, and @markus.blatt.
- Just leave it on master for now and apply any changes there.
mentioned in merge request !71 (merged)
-
Two people have commented so far (@gruenich: !16 (comment 15545), @markus.blatt on dune-devel). It looks like the suggestion is to just leave it as it is and to mop up any problems that may come up. Which I will of course do.
-
@felix.gruber noticed a problem and made a patch in !73 (closed).
@oliver.sander feels that it might be better to revert dune-istl!17 (comment 15640) -- so I'm reopening.
-
I'm a bit concerned about the possibility of merging the branch later, after a revert. Simply merging the branch again would be a noop, I think, since git would take the later reverts as authoritative and resolve to the reverted state.
I guess you could revert the revert, but that seems horribly ugly. Or rebase the branch onto the reverts and merge that, but I seem to recall that people were quite heavily opposed to visible rebasing. But then, this seems like an exceptional situation.
Alright, I'm going to test the revert+rebase solution locally and report back.
-
The straightforward rebasing onto the revert does not work (it just fast-forwards onto the the revert). Luckily, someone has documenten how to do it right.
-
There seem to be two ways to do this:
- revert the offending merge on master, continue developing on feature/vc, and if the time comes, revert the revert on master, then merge feature/vc. This has the obvious disadvantage that we must remember to revert the revert just before the commit. Also, I am not sure what gitlab will make of that. It has the advantage that the people using the branch don't need to reset their branch or switch to another branch.
- revert the offending merge on master, recreate the branch with fresh commit ids as feature/vc-2, continue developing on feature/vc-2, and if the time comes simply merge feature/vc-2 into master. This has the advantage that merging won't be any more complicated then usual, but people must switch branches now.
I tried the second option, since that seems to be less fragile.
- check out dune-common
- the faulty merge is 68d8b5a5. This is not the actual merge into master, but into my sidebranch feature/doc-building-of-tests that was in turned merged into master.
- the starting point of feature/vc is 5438c22b.
-
git checkout -b feature/vc-2 origin/feature/vc && git rebase --no-ff 5438c22creates a copy of feature/vc with different commit ids. -
git checkout -b feature/revert-vc origin/master && git revert -m 1 68d8b5acreates a feature branch with the faulty merge reverted, ready to be merged onto master. I checked that merging first feature/revert-vc and then feature/vc-2 into master has not net effect on the contents of the repository tree.
mentioned in commit 474b5e9f
mentioned in merge request !74 (merged)
-
I just created a merge request !74 (merged) for the revert and !75 (closed) for futher development on feature/vc-2.
TODO:
- Wait for rough consensus.
- merge !74 (merged).
- mention in !16 (merged) that the discussion should continue in !75 (closed).
- point dune-istl!17 (merged) to !75 (closed).
- delete feature/vc.
- cherry-pick !73 (closed) on top of feature/vc-2
@christi, @markus.blatt: Are you OK with this? You are going to need to rebase anythin that you have on top of feature/vc onto feature/vc-2.
Edited by Jö Fahlke -
@all So far I haven't received any comments on the proposed plan. I don't want to rush this, but I'm going to be on holidays starting Tuesday next week (2016-03-22) for about two weeks. So I'm going to apply the revert to master an Wednesday this week (2016-03-16), that way I have still some days for firefighting should something go wrong yet again. I currently see no way around the revert anyway.
@christi, @markus.blatt: I think the decision about what to do with the branch is less pressing, I'm going to try and discuss that with @christi when hes back on monday, but I think it would not be the end of the world if that is delayed further.
mentioned in commit 5afd88aa
-
@christi prefers if the name of the branch stays the same. I haven't had any other comments, and we don't think anyone else is using the branch anyway, and I'd like to finish this before I go on holidays tomorrow, so I'm not going with the above plan, but will force-push feature/vc-2 over feature/vc.
So the new plan is:
- Cherry-pick !73 (closed) on top of feature/vc-2 (Already done by @felix.gruber, see !73 (comment 15950), thanks)
- Force-push feature/vc-2 as feature/vc.
- Close !75 (closed) with an appropriate comment, deleting feature/vc-2
Edited by Jö Fahlke -
Force-pushing feature/vc-2 as feature/vc kind-of worked -- what did not work is that MR!16 remains marked as merged, So we need a new merge request for discussion.
New todo-list:
- Make new MR for feature/vc, as a followup to !16 (merged).
- Point !16 (merged) to the new MR.
- Point dune-istl!17 (merged) to the new MR.
- Find the dune-geometry MR and point to to the new MR. (dune-geometry!13 (closed)).
- Close !75 (closed) and delete feature/vc-2
Edited by Jö Fahlke mentioned in merge request !81 (merged)
-
The new MR is !81 (merged).
mentioned in commit c955776c
Mentioned in issue flyspray/FS#1669 (closed)
Mentioned in commit c955776c
Mentioned in commit 474b5e9f
Mentioned in commit 5afd88aa
