The delang is using merge instead of rebase/squash

deadalnix via Digitalmars-d digitalmars-d at puremagic.com
Tue Mar 21 04:59:42 PDT 2017


On Tuesday, 21 March 2017 at 01:39:39 UTC, Vladimir Panteleev 
wrote:
> On Monday, 20 March 2017 at 12:25:22 UTC, deadalnix wrote:
>> Because a picture is clearer than a thousand words:
>
> What this tells me is that the default way git-log presents 
> history is not very useful. Consider this presentation of the 
> same information:
>

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 ?

> In particular, the origin commit of a branch is often not 
> interesting; only the list of commits that are on one branch 
> and aren't on another are.
>

Yes, that's why rebasing makes thing clearer. Nobody care what 
the master commit was when the work was started.

>> First there is no given that any intermediate state is sound, 
>> or even builds at all. That makes it very hard to bissect 
>> anything.
>
> Bisecting D is not something that can be reasonably done by 
> looking at just one repository's history anyway; this is why we 
> have D-dot-git and Digger. Either way, for pull requests that 
> make non-trivial changes or additions, you will need to descend 
> into the pull request itself.
>

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

While I agree with you that things like bisecting are broken in 
D, I don't see it as a reason to screw things up even more. I'm 
not a big fan of "it's already broken, so we can break it even 
more". This should, and can, be fixed.

https://danluu.com/monorepo/

Incidentally, I got a company contacting me last week willing to 
pay me good money to help them transition toward these kind of 
workflow.

> - If a pull request that should not have been squashed has been 
> squashed while merging, the result is:
>   - Commit messages are lost and remain available only on 
> GitHub.
>   - Any logical separation of changes that might have been 
> represented through separate commits is lost and remains 
> available only on GitHub.
>   - "git blame" becomes less useful because it can only lead to 
> the big blob of the squashed changes.
>   - "git blame" becomes less useful because in some situations 
> it loses its ability to track moved code, which should and 
> often is done in separate commits.
>   - Bisection becomes more difficult because it is no longer 
> easily possible to dive into a PR, as has been occasionally 
> necessary.
>

Then it should have been 2 PR or more to begin with. Splitting PR 
in smaller ones is a good practice in general, there are ample 
proof that is increase the quality of the code review, reduce 
conflicts surface with other PR, makes reverting easier and more 
targeted when something happens, etc...

Keeping this PR's commits is just a way to mitigate one of the 
negative consequences of kitchen sink PRs. It does so by 
impacting negatively others aspects of source control, and does 
nothing to mitigate other negatives aspects of kitchen sink PRs, 
such as review fatigue (see a specific example below).

> In general, I am not opposed to giving reviewers the option to 
> merge pull requests with squashing, assuming we can all agree 
> to not abuse it and only use it for PRs where there nothing 
> useful can be gained by preserving the multiple commits as they 
> are; however, their words and actions have shown that this 
> doesn't seem to be an attainable point of agreement.

If multiple commits are important for the PR, then the PR should 
have been several PR to begin with. Asking people to split s the 
way to go.

Consider this PR: 
https://github.com/BitcoinUnlimited/BitcoinUnlimited/pull/164

You can see in the comments that I asked the original author to 
split it up because it was a kitchen sink and very hard to review 
in its current form. This was ignored. The PR ended up containing 
a bug that would cost about $12 500 to one of the users of the 
software, plus a fair amount of reputational damage. The change 
containing the bug did not need to be bundled with the rest of 
the PR, and would have almost certainly be noticed if it had been 
made on a PR of its own.

Bundling several changes in the same PR has real world 
consequences that go beyond screwing up source control.



More information about the Digitalmars-d mailing list