Re: fast forward merge overwriting my code

On 24/05/2021 08:13, Andre Ulrich wrote:

I think that last point highlights the issue you guys are having - using `merge` for doing both (1) actual merge, but also (2) review and edit at the same time, which is wrong (or very unconventional, to say the least).

In ideal case (meaning no conflicts, no matter if 3-way merge or a fast-forward one), merge should accept all the changes being merged in from the side branch and incorporate them into the main branch.

From this basic and the most common scenario alone it is visible that merge should not "keep some changes, but not all" - the very point of a merge is to (try to) keep _all the changes_, period.

Now, as for the "try to" part - in some cases not all changes can be kept as they are, like when both branches changed same files and same lines (or close to), so that's when Git hands over the resolution to the user, to determine what is the desired outcome of a conflicting merge.

Still, even in this case, the final outcome should be considered a sum of all the changes, even though some might have been altered or rearranged in order to better work with each other (as different branches might have done the same thing in a different way).

In any case, it should not be up to the merge (process nor commit) to discard (nor add!) some of the non-conflicting changes you have made on your 'update' branch - it is possible to do (something usually called an "evil merge", and for a reason), yet is not a good practice.

As an example, imagine you have commits 1, 2 and 3 on your 'update' branch, and upon merging your professor decides to accept changes from commit 2 only, completely discarding changes from commits 1 and 3. Your history will end up looking something like this:

(1) ---X-----------M 'master'
        \         /
         1---2---3 'update'

... where M is the merge commit, merging branch 'update' into 'master'. As it is, it's reasonable to expect of M to contain all the changes brought in by 1, 2 and 3 - yet it is not the case, which could be rather confusing (on later history review).

What would be a more common/usual scenario is, after trying a local merge M and seeing some changes should not be accepted (like commits 1 and 3), have your professor communicate the problem with you so you can fix the issues yourself, inside 'update' branch, and iteratively repeat this process as long as 'update' branch is not "perfect" - at which point it can be accepted _as a whole_, that is.

You professor should not accept to merge your changes as long as they are not all correct, and he specifically should not be using the merge to correct the issues himself.

Depending on your preference, he _could_ be doing the changes himself, too - but again doing so through standalone commits (on your 'update' branch, for example), _not_ through a merge commit.

Based on example (1) above, the finally merged changes history could instead look like this:

(2) ---X-------------------M1 'master'
        \                 /
         1---2---3---4---5 'update'

..., where commits 4 and 5 are fixes made on 'update' after your professor's comments on commits 1, 2 and 3, and M1 is the merge which finally accepts all the changes from 'update'.

Alternatively, if you use rebase, you can alter problematic commits 1 and 3 directly instead, so the history would look something like this:

(3) ---X-----------M2 'master'
        \         /
         1'--2'--3' 'update'

..., where original commits 1 and 3 are changed in order to be acceptable for the merge, becoming commits 1' and 3', while commit 2' would stay the same as original commit 2. Again, merge commit M2 accepts all the changes as they now are (all correct).

Also, if commits 1 and 3 are completely wrong and not required in the first place, yet another alternative (using rebase) would be to drop them altogether, ending up with a history like this:

(4) ---X----M3 'master'
        \  /
         2" 'update'

..., where commit 2" would be exactly the same as original commit 2, and commits 1 and 3 are dropped from the history completely - and transparently, _not_ using the merge to do so, as in original example (1) (and your explained scenario).

I hope these examples somewhat help, the main point remaining that merge should not be used to discard/disapprove certain (especially non-conflicting) changes, but only to finally accept/approve _all_ the changes, possibly modified in the meantime as a result of an iterative review and additional work.

Note that there's nothing wrong in having your professor do his own local merges as part of this review process, but those should be only temporary, to be discarded and not accepted until everything can be merged (and accepted) as-is.

Regards, Buga