coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Multibyte support for sort, uniq, join, tr, cut, paste, expand, unex


From: Kamil Dudka
Subject: Re: Multibyte support for sort, uniq, join, tr, cut, paste, expand, unexpand, fmt, fold, and pr
Date: Wed, 21 Mar 2018 12:11:48 +0100

On Tuesday, March 20, 2018 11:18:23 PM CET Assaf Gordon wrote:
> Two things for later (not critical for now):
> to make review easier, it's recommended to combine all commits that
> relate to a single program
> into one commit. This is called "squash" in git (see:
> http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html
> https://blog.carbonfive.com/2017/08/28/always-squash-and-rebase-your-git-com
> mits/ ).

I strongly disagree with the statement above.  If the changes are reasonably 
split into mlutiple commits per a single program such that coreutils can be 
built and run them after applying each single commit, then squashing them into 
bigger commits is strictly counterproductive.

If some of the changes introduces a regression, which is not so unlikely in 
this case, one can use git-bisect to find the commit that broke something.  It 
will return the first bad commit and, if the corresponding change is small,
it will immediately tell us what went wrong.  But if the first bad commit is
a huge block of all multibyte-related changes for a single utility, you are
on your own and need to find the mistake manually.

Please have a look at it from the other side.  For you as a reviewer, it is 
trivial to view diff over several commits, or squash them locally.  On the 
other hand, squash is irreversible operation.  Once you push unreasonably
huge commits, they cannot be later automatically split into several smaller 
commits in a sensible way.

I am, of course, not talking about commits like "fix the previous change to 
make it compile", "fix typo in a comment" and the like -- those should really 
by squashed.

Kamil



reply via email to

[Prev in Thread] Current Thread [Next in Thread]