On Tuesday, 21 March 2017 at 12:45:45 UTC, Vladimir Panteleev wrote:
> On Tuesday, 21 March 2017 at 11:59:42 UTC, deadalnix wrote:
>> It's not good either. Why would I want to look at a DAG when the serie of event is strictly linear to begin with ?
>
> Not sure what you mean here. The way it's presented is not a DAG.
>

Blue is red, up is down, and the commit graph is not a DAG.

>> "Our source control is completely broken, but that's not a problem because we developed 3rd party tools to work around the brokenness"
>
> That's fallacious.
>

If you can't bissect, it's broken.

Listen, you know it's broken because you wrote tools to work around the brokenness. If it wasn't broken you wouldn't have written these tools as there would be no need to do so. So let's not play pretend.

On Tuesday, 21 March 2017 at 12:49:22 UTC, Vladimir Panteleev wrote:
>> there are ample proof that is increase the quality of the code review,
>
> OK, where is the proof?

Large companies such as Google or Facebook measure these things.

You have presented 0 arguments so far, and dismissed both facts and argument that were presented to you (one of them as unfair, because fairness and correctness surely are correlated).

But cool guys you are right, don't change anything. This is great. I have other things to do to convince you guys when other are paying me to do so.

On Monday, 20 March 2017 at 05:10:04 UTC, Martin Nowak wrote:
> On Wednesday, 15 March 2017 at 13:14:31 UTC, deadalnix wrote:
>> This is making the history very spaghettified. Is that possible to have the bot rebase/squash commits and then pushing ?
>
> I don't really agree with the argument. A merge commit is a clear way to integrate changes from a PR/branch.
> Just rebasing a PR on top of master removes a lot of information from git, only leaving references to github.
>
> Can you be more specific, what you mean w/ spaghetti? The fact that reciew fixes are added to PRs.
>
> Also github's commit view misleadingly shows commits from merged PRs/branches, which aren't actually in master.

There's a compromise, which I'm using right now. Always rebase and always merge. You can see that it branched off for a purpose (the argument for merging) and the history is much cleaner (the argument for rebasing).

i.e.:

git rebase master my_branch
git checkout master
git merge --no-ff my_branch

gitlab supports doing this via the web interface, I don't know about github.

Atila

On Tuesday, 21 March 2017 at 18:07:57 UTC, deadalnix wrote:
> You have presented 0 arguments so far, and dismissed both facts and argument that were presented to you (one of them as unfair, because fairness and correctness surely are correlated).

This is factually wrong, as is obvious from reading the thread.

If you are not interested in constructive discussion, then I'm sorry that both of our times have been wasted.

On Tuesday, 21 March 2017 at 17:58:06 UTC, deadalnix wrote:
> On Tuesday, 21 March 2017 at 12:45:45 UTC, Vladimir Panteleev wrote:
>> On Tuesday, 21 March 2017 at 11:59:42 UTC, deadalnix wrote:
>>> It's not good either. Why would I want to look at a DAG when the serie of event is strictly linear to begin with ?
>>
>> Not sure what you mean here. The way it's presented is not a DAG.
>
> Blue is red, up is down, and the commit graph is not a DAG.

Not sure what you mean. The commit graph is a DAG. The way you quoted my post made your remark seem to refer to my attempt to reformat it, which is not presented as a DAG.

>>> "Our source control is completely broken, but that's not a problem because we developed 3rd party tools to work around the brokenness"
>>
>> That's fallacious.
>>
>
> If you can't bissect, it's broken.

By that definition of "broken", all git repositories which use branch merging are "broken". That includes some of the biggest open-source projects. Frankly, if you want to stick to that definition, I have nothing against it.

> Listen, you know it's broken because you wrote tools to work around the brokenness. If it wasn't broken you wouldn't have written these tools as there would be no need to do so. So let's not play pretend.

Digger would probably have existed even if D were a monorepo and squashed PRs' commits from the start, because it also knows how to satisfy each prior version's build dependencies and how to invoke the build scripts. Regardless, D is perfectly suitable for automatic bisection, which is unreasonably awkward with git itself - Digger makes it much easier. I think there's no shame in writing domain-specific tools to enhance some functionality of standard ones.

It's common practice for "merge" commits to have the form:
"merge work from some/branch, fix PR #somenumber".

This basically tells me nothing about what this commit does.

We already know it's a merge commit, we don't care so much what branch it's from, and we don't want to dig into the bug tracker to translate the issue number into english.

We care more about how this merge modifies the code behaviour.

What if "merge" commits had better messages, not containing the word "merge" at all?
This way, the depth-0 history, which is always linear, would be human-readable and bisectable.

On Wednesday, 22 March 2017 at 01:25:37 UTC, Vladimir Panteleev wrote:
> On Tuesday, 21 March 2017 at 17:58:06 UTC, deadalnix wrote:
>> On Tuesday, 21 March 2017 at 12:45:45 UTC, Vladimir Panteleev wrote:
>>> On Tuesday, 21 March 2017 at 11:59:42 UTC, deadalnix wrote:
>>>> It's not good either. Why would I want to look at a DAG when the serie of event is strictly linear to begin with ?
>>>

This is almost human readable...
git log --first-parent --no-merges --decorate
... except if a merge commit is tagged, I haven't found any solution for that, can you? It's very important to be able to see tags, yet filter away merge commits.

Fortunately I managed to convert my team to rebase, so I no longer suffer this problem at work, only with D.

Even this simplest git commands break down:
git show

WTF? there was no difference? Ahh... I was supposed to type:
git show --first-parent
well at least this case can be solved by a simple alias, but log cannot.

On Tuesday, 21 March 2017 at 18:07:57 UTC, deadalnix wrote:
> Large companies such as Google or Facebook

A blind appeal to authority is fallacious, but it's still worthwhile to see what others are doing. I think it's important to look at projects that are similar to our own, so I looked at what other programming language implementations do.

- Go is developed using Google's source code infrastructure, and code reviews happen using Gerrit. On Gerrit, every commit is reviewed separately (as I've been advocating). Furthermore, if you push multiple commits to Gerrit, this automatically creates one review page per commit, and marks them as inter-dependent in the commit order. This is an awesome approach, and I wish GitHub made this workflow more practical. Importantly, Gerrit does not squash commits - you are expected to squash fixup commits yourself.

- Rust uses GitHub, and all merges seem to be done by a bot. We are heading in that direction too. The bot uses regular merges and does not squash commits or rebase them onto master.

- Python: I looked at the CPython repository on GitHub. They seem to be using squashing exclusively, and only using branches for version maintenance. However, when I tried to find how they would deal with a contribution that would be desirable to be split into several PRs/commits, I couldn't find one on the first 5 pages of merged PRs. I guess the project is in the stage of mostly minor bugfixes only - we're certainly not there yet.

  Curiously, submitters are expected to resubmit the same PR themselves against every maintenance branch, e.g. here is the same PR submitted 4 times, to different branches:

  - https://github.com/python/cpython/pull/629
  - https://github.com/python/cpython/pull/633
  - https://github.com/python/cpython/pull/634
  - https://github.com/python/cpython/pull/635

- Ruby uses Subversion, a GitHub mirror, and a bot which synchronizes between the two. I don't think there's anything we can learn from here.

- OCaml uses GitHub PRs and regular git merges.

- Clang and GHC use Phabricator. I'm not too familiar with it, but I understand it's not too different from Gerrit: it creates one review per commit, and you can push multiple commits at once which will do the right thing.

To sum it up, I don't think we're doing anything too weird. Though it would be nice if GitHub's UI were to improve to better handle this workflow, I don't think it makes sense to force submitters to go through the busywork of creating one PR per commit for many cases.

On Wednesday, 22 March 2017 at 09:02:24 UTC, Vladimir Panteleev wrote:
> Importantly, Gerrit does not squash commits - you are expected to squash fixup commits yourself.

You can configure Gerrit to do virtually anything, including squashing, even cherry-pick if you fancy.

On Wednesday, 22 March 2017 at 10:43:46 UTC, Daniel N wrote:
> On Wednesday, 22 March 2017 at 09:02:24 UTC, Vladimir Panteleev wrote:
>> Importantly, Gerrit does not squash commits - you are expected to squash fixup commits yourself.
>
> You can configure Gerrit to do virtually anything, including squashing, even cherry-pick if you fancy.

Ah, thanks. Could you link me to the relevant documentation? Looking at https://bugs.chromium.org/p/gerrit/issues/detail?id=1254, it seems Gerrit can't handle multiple commits in any scenarios right now.

Either way, I guess it doesn't squash a series of inter-dependent commits/reviews into one :)