Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
On Thu, Oct 17, 2019 at 4:24 PM Roman Gilg wrote: > > On Tue, Oct 15, 2019 at 8:12 PM Ben Cooksley wrote: > > > > On Wed, 16 Oct 2019, 05:17 Johan Ouwerkerk, wrote: > >> [...] > > > > > > It was complexity of that degree that I was primarily concerned about when > > people started pushing for being able to force push and use a rebase > > workflow. > > > > For a first time contributor or anyone who isn't familiar with working with > > Git rebase, that sort of workflow is incredibly daunting, downright scary > > and is likely to push them in the direction of simply not contributing at > > all. > > > > As I'm sure we can all agree, that isn't something we want at all - we want > > it to be easy and straight forward to get involved for everyone. > > > > In cases such as the one you have above, I think a simple merge commit > > would be completely fine in that instance, especially given it is an > > uncommon event. > > It was like this on Phabricator until now so we should leave it like > that for the GitLab transition. > The rebase behaviour described by Johan is something you would need to do manually with feature branches, while the Gitlab behaviour in question in this thread is a fully automated one. That makes it quite different from what we had with Phabricator. (Side note: Phabricator doesn't carry commits, so anything landed is always squashed) > That being said the git merge workflow instead of rebase has certain > advantages and just because we did it until now with rebase does not > need to imply we have to continue to do so for all time. > > What I mean by that: if you set the default of GitLab merge behavior > to rebase, please do it in a way that allows individual projects to > override the default at some point in the future if seen fit. As noted previously, Gitlab does not allow setting a default for new projects - so what we will do is change the setting on all of our projects, and ensure we change it on all new projects going forward. I'm not sure whether we should support/permit projects to opt to a different workflow, as it is bound to cause someone to notice it is different and then file a Sysadmin ticket saying it's wrong because it is different to the policy applied to all other projects. This means that we (Sysadmin) then have to track who has opted to have different behaviour, and people are then bound to challenge why it is different when we close those tickets as invalid. Regards, Ben
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
On Tue, Oct 15, 2019 at 8:12 PM Ben Cooksley wrote: > > On Wed, 16 Oct 2019, 05:17 Johan Ouwerkerk, wrote: >> [...] > > > It was complexity of that degree that I was primarily concerned about when > people started pushing for being able to force push and use a rebase workflow. > > For a first time contributor or anyone who isn't familiar with working with > Git rebase, that sort of workflow is incredibly daunting, downright scary and > is likely to push them in the direction of simply not contributing at all. > > As I'm sure we can all agree, that isn't something we want at all - we want > it to be easy and straight forward to get involved for everyone. > > In cases such as the one you have above, I think a simple merge commit would > be completely fine in that instance, especially given it is an uncommon event. It was like this on Phabricator until now so we should leave it like that for the GitLab transition. That being said the git merge workflow instead of rebase has certain advantages and just because we did it until now with rebase does not need to imply we have to continue to do so for all time. What I mean by that: if you set the default of GitLab merge behavior to rebase, please do it in a way that allows individual projects to override the default at some point in the future if seen fit.
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
El dimarts, 15 d’octubre de 2019, a les 9:16:48 CEST, Frederik Schwarzer va escriure: > > Am 14.10.2019 22:51 schrieb Johan Ouwerkerk: > > On 14.10.2019 21:22, Frederik Schwarzer wrote: > >> If however, master had seen commits as well, fast-forwarding is > >> performing a rebase ... is that correct? > > > > The workflow would be: whenever master is updated, you rebase your > > local feature/work branch and force-push to the remote copy of the > > feature/work branch. > > This is exactly the problem I see. > I create a branch. > I start to use, let's say ... KDialog in my feature as KDialog has been > used throughout the application and make 20 commits. > Now on master, someone merges a branch that replaces all the KDialogs > with overlays and removes all KDialog includes. > So if I rebase on that, all my 20 commits will fail to build. Checking > out an older revision to test something will not work. > Now I will fix my latest revision and merge to master. Still: 19 commits > are not compiling anymore. > > Or am I missing something here? > > How would we deal with that? Is "short-lived branches" (as you stated > below) enough to reduce the risk? You have the same problem with arc, arc rebases, so if we had no problem until now, we have no problem now. Cheers, Albert > > Cheers > Frederik > > >
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
On Wed, 16 Oct 2019, 05:17 Johan Ouwerkerk, wrote: > On Tue, Oct 15, 2019 at 9:17 AM Frederik Schwarzer > wrote: > > Now I will fix my latest revision and merge to master. Still: 19 commits > > are not compiling anymore. > > > > Or am I missing something here? > > > > How would we deal with that? Is "short-lived branches" (as you stated > > below) enough to reduce the risk? > > > > To answer in reverse order: > > Yes. On the one hand short-lived branches reduce this risk > considerably: this scenario applies to breaking changes in master > which fundamentally alter the way the application works. It's like a > core library upgrade or, in this case, a major UX rewrite: something > fairly fundamental to the application changes in master. It is > unlikely that this should happen overnight without any kind of prior > review. > > Still, this can and does happen and will happen some day to you if you > contribute enough :) However this gets back at the git rebase bit. > > So yes, you rebase your feature on master as per normal, fix transient > merge conflicts and then what? Well, then you still have to compile & > test. At which point you notice breakage. How do you recover? As you > would: you begin the porting effort, either changes from master to > your new feature way of doing things (in case *you* are the one doing > the UX rewrite/major refactoring), or vice versa you apply the new > world order from master to your feature. > > What I like to do during this process is to avoid committing these > fixes just yet. I want to get a feel for the total diff, in particular > the total git diff --stat that I accumulate. Then I can identify on a > file-by-file basis using something like git log -3 path/to/file or so > what the likely commit is which should have been amended. Sometimes > you notice the diff for a file should be spread over multiple commits > according to your prior log, so what you do next is you use git add > like this: git add -p path/to/file. You only select the bits for which > you have identified a particular commit, you commit those added hunks > and here I like to leave a note in the first line of the commit to the > effect of "fixup " or "squash " or "". > > In this way you build up a bunch of commits which cover your fixes. > Next up, you turn to git rebase again, using e.g. git rebase -i > master. Now you can interactively fold the commits into the history as > "it ought to be" and this is where I use my notes to help me decide > how to proceed. Note you don't have to get everything just right, and > note that this rebasing itself may introduce transient merge conflicts > you need to fix: so if the diff stat was large it makes sense to split > this up into multiple git rebase -i runs just to give yourself a break > in between. Finally perhaps you rebase again to touch up a few commit > messages or something, and if this whole process took considerable > amount of time you want to verify that upstream master has not yet > moved on by that point. > > So in this more complex case you can adopt a correspondingly more > complex git workflow and use rebase to produce clean commits. Now, > sometimes you decide this is all too much work, and too much bother. > What you can do in such a scenario instead is to create a fresh new > branch from master and effectively re-create commits there. In those > cases git cherry-pick and git checkout -p -- path/to/file > come in handy > > Ultimately whether or not a scrupulously clean commit log is worth the > effort or whether you might decide to simplify things a little and > accept a few broken commits in between mostly depends on the needs of > your project and how many people work on it. > It was complexity of that degree that I was primarily concerned about when people started pushing for being able to force push and use a rebase workflow. For a first time contributor or anyone who isn't familiar with working with Git rebase, that sort of workflow is incredibly daunting, downright scary and is likely to push them in the direction of simply not contributing at all. As I'm sure we can all agree, that isn't something we want at all - we want it to be easy and straight forward to get involved for everyone. In cases such as the one you have above, I think a simple merge commit would be completely fine in that instance, especially given it is an uncommon event. > Regards, > > -Johan > Cheers, Ben >
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
On Tue, Oct 15, 2019 at 9:17 AM Frederik Schwarzer wrote: > Now I will fix my latest revision and merge to master. Still: 19 commits > are not compiling anymore. > > Or am I missing something here? > > How would we deal with that? Is "short-lived branches" (as you stated > below) enough to reduce the risk? > To answer in reverse order: Yes. On the one hand short-lived branches reduce this risk considerably: this scenario applies to breaking changes in master which fundamentally alter the way the application works. It's like a core library upgrade or, in this case, a major UX rewrite: something fairly fundamental to the application changes in master. It is unlikely that this should happen overnight without any kind of prior review. Still, this can and does happen and will happen some day to you if you contribute enough :) However this gets back at the git rebase bit. So yes, you rebase your feature on master as per normal, fix transient merge conflicts and then what? Well, then you still have to compile & test. At which point you notice breakage. How do you recover? As you would: you begin the porting effort, either changes from master to your new feature way of doing things (in case *you* are the one doing the UX rewrite/major refactoring), or vice versa you apply the new world order from master to your feature. What I like to do during this process is to avoid committing these fixes just yet. I want to get a feel for the total diff, in particular the total git diff --stat that I accumulate. Then I can identify on a file-by-file basis using something like git log -3 path/to/file or so what the likely commit is which should have been amended. Sometimes you notice the diff for a file should be spread over multiple commits according to your prior log, so what you do next is you use git add like this: git add -p path/to/file. You only select the bits for which you have identified a particular commit, you commit those added hunks and here I like to leave a note in the first line of the commit to the effect of "fixup " or "squash " or "". In this way you build up a bunch of commits which cover your fixes. Next up, you turn to git rebase again, using e.g. git rebase -i master. Now you can interactively fold the commits into the history as "it ought to be" and this is where I use my notes to help me decide how to proceed. Note you don't have to get everything just right, and note that this rebasing itself may introduce transient merge conflicts you need to fix: so if the diff stat was large it makes sense to split this up into multiple git rebase -i runs just to give yourself a break in between. Finally perhaps you rebase again to touch up a few commit messages or something, and if this whole process took considerable amount of time you want to verify that upstream master has not yet moved on by that point. So in this more complex case you can adopt a correspondingly more complex git workflow and use rebase to produce clean commits. Now, sometimes you decide this is all too much work, and too much bother. What you can do in such a scenario instead is to create a fresh new branch from master and effectively re-create commits there. In those cases git cherry-pick and git checkout -p -- path/to/file come in handy Ultimately whether or not a scrupulously clean commit log is worth the effort or whether you might decide to simplify things a little and accept a few broken commits in between mostly depends on the needs of your project and how many people work on it. Regards, -Johan
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
On Вт, окт 15, 2019 at 09:16, Frederik Schwarzer wrote: Am 14.10.2019 22:51 schrieb Johan Ouwerkerk: On 14.10.2019 21:22, Frederik Schwarzer wrote: If however, master had seen commits as well, fast-forwarding is performing a rebase ... is that correct? The workflow would be: whenever master is updated, you rebase your local feature/work branch and force-push to the remote copy of the feature/work branch. This is exactly the problem I see. I create a branch. I start to use, let's say ... KDialog in my feature as KDialog has been used throughout the application and make 20 commits. Now on master, someone merges a branch that replaces all the KDialogs with overlays and removes all KDialog includes. So if I rebase on that, all my 20 commits will fail to build. Checking out an older revision to test something will not work. Now I will fix my latest revision and merge to master. Still: 19 commits are not compiling anymore. Or am I missing something here? How would we deal with that? Is "short-lived branches" (as you stated below) enough to reduce the risk? Well, usually in a situation like that you, being the author, would know that previous 19 commits needs fixing as well as the 20th one. Besides, I think when people would start review, they will probably notice some odd discrepancy between commits 19 and 20. Or someone just would remember while reviewing nth commit "Hey, this not gonna work due to recent changes in codebase". But yeah, ultimately it's up to you. FWIW: I seem to recall there's a way to enable automatic merge of a branch once CI passes. However, running CI for every commit is not implemented yet https://gitlab.com/gitlab-org/gitlab/issues/25148 Not that I think it's worth to enable though… The situation you describe seems rare to me, but it's my subjective opinion, I might be wrong.
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
Am 14.10.2019 22:51 schrieb Johan Ouwerkerk: On 14.10.2019 21:22, Frederik Schwarzer wrote: If however, master had seen commits as well, fast-forwarding is performing a rebase ... is that correct? The workflow would be: whenever master is updated, you rebase your local feature/work branch and force-push to the remote copy of the feature/work branch. This is exactly the problem I see. I create a branch. I start to use, let's say ... KDialog in my feature as KDialog has been used throughout the application and make 20 commits. Now on master, someone merges a branch that replaces all the KDialogs with overlays and removes all KDialog includes. So if I rebase on that, all my 20 commits will fail to build. Checking out an older revision to test something will not work. Now I will fix my latest revision and merge to master. Still: 19 commits are not compiling anymore. Or am I missing something here? How would we deal with that? Is "short-lived branches" (as you stated below) enough to reduce the risk? Cheers Frederik
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
On 14.10.2019 21:22, Frederik Schwarzer wrote: > If however, master had seen commits as well, fast-forwarding is > performing a rebase ... is that correct? The workflow would be: whenever master is updated, you rebase your local feature/work branch and force-push to the remote copy of the feature/work branch. That way you would get a linear commit history whenever you merge the feature/work branch to master. People reviewing your work should be prepared to reset --hard to the origin whenever that happens. There are two caveats with this model: - one: you should never accept MRs which are (partially) behind master, because the person who merges things in should not modify what is being merged in. Rather the person who proposes the merge should take the time to do the necessary rebase, because they know best how to do that. - especially when working with many people on the same code-base there should be reasonably fast paced dev -> MR -> review -> merge cycle. Long lived feature/work branches are really bad because as time goes on it becomes fairly difficult to keep them up to date. Regards, -Johan
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
On 14.10.2019 21:22, Frederik Schwarzer wrote: Hi, just asking in case I didn't get it. I branch off of master and do a few commits in that new branch. If I now merge the branch back to master and master had not seen any commits in between, it's just relocating the master "tag" and all is fine. If however, master had seen commits as well, fast-forwarding is performing a rebase ... is that correct? Yes Wouldn't rebasing be evil because it rewrites history? Well, it only rewrites your personal branch. Just don't tell anybody :) In all seriousness though: it's not specific to gitlab, you're supposed to rewrite history to address code review anyway. It would be the same even with mailing-list-managed projects: somebody says "No, it would be better if 3-rd and 4-th patches would be joined into one", and now to do it you need to rewrite your local commits history.
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
Hi, just asking in case I didn't get it. I branch off of master and do a few commits in that new branch. If I now merge the branch back to master and master had not seen any commits in between, it's just relocating the master "tag" and all is fine. If however, master had seen commits as well, fast-forwarding is performing a rebase ... is that correct? Wouldn't rebasing be evil because it rewrites history? Cheers Frederik On 10/14/19 6:29 PM, Johan Ouwerkerk wrote: Yes, please, pretty please with cherry on top. :) Regards, -Johan On Sun, Oct 13, 2019 at 10:57 PM Albert Astals Cid wrote: I find the merge behavior to be not what we've been doing in phabricator so given the idea is to maintain our workflows i'd appreciate if we can agree on continue doing the same. https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html Opinions? Cheers, Albert
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
Yes, please, pretty please with cherry on top. :) Regards, -Johan On Sun, Oct 13, 2019 at 10:57 PM Albert Astals Cid wrote: > > I find the merge behavior to be not what we've been doing in phabricator so > given the idea is to maintain our workflows i'd appreciate if we can agree on > continue doing the same. > > https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html > > Opinions? > > Cheers, > Albert > >
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
On zondag 13 oktober 2019 22:57:20 CEST Albert Astals Cid wrote: > I find the merge behavior to be not what we've been doing in phabricator so > given the idea is to maintain our workflows i'd appreciate if we can agree on > continue doing the same. > > https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html > > Opinions? > We made that the default for Krita as well. -- Boudewijn Rempt | https://www.valdyas.org | https://www.krita.org signature.asc Description: This is a digitally signed message part.
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
On Пн, окт 14, 2019 at 02:42, Aleix Pol wrote: On Sun, Oct 13, 2019 at 10:57 PM Albert Astals Cid wrote: I find the merge behavior to be not what we've been doing in phabricator so given the idea is to maintain our workflows i'd appreciate if we can agree on continue doing the same. https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html +1 This is the enterprise version though, does this apply to the community? Adding sysadmin as CC, since they're the ones who will have to put it to work in the end. Yeah, it works in community edition too, we use it at work.
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
On Sun, Oct 13, 2019 at 10:57 PM Albert Astals Cid wrote: > > I find the merge behavior to be not what we've been doing in phabricator so > given the idea is to maintain our workflows i'd appreciate if we can agree on > continue doing the same. > > https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html +1 This is the enterprise version though, does this apply to the community? Adding sysadmin as CC, since they're the ones who will have to put it to work in the end. Aleix
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
+ 1 Commit with the name "Merge branch 'branch-name' into 'master' are not helpful. Cheers, Carl On Sunday, October 13, 2019 10:57:20 PM CEST Albert Astals Cid wrote: > I find the merge behavior to be not what we've been doing in phabricator so > given the idea is to maintain our workflows i'd appreciate if we can agree > on continue doing the same. > > https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.ht > ml > > Opinions? > > Cheers, > Albert publickey - Description: application/pgp-key signature.asc Description: OpenPGP digital signature
Re: Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
+1 On Sun, 13 Oct 2019, 22:58 Albert Astals Cid, wrote: > I find the merge behavior to be not what we've been doing in phabricator > so given the idea is to maintain our workflows i'd appreciate if we can > agree on continue doing the same. > > > https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html > > Opinions? > > Cheers, > Albert > > >
Can we agree to change gitlab default behaviour from merge to fast-forward merge for all repos?
I find the merge behavior to be not what we've been doing in phabricator so given the idea is to maintain our workflows i'd appreciate if we can agree on continue doing the same. https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html Opinions? Cheers, Albert
