[RFC] collapse commits in branches to easy review

[RFC] collapse commits in branches to easy review

[Koen Kooi]

Hi,

I want to propose that when people want to have a branch reviewed for 
merging that has a lot of commits (e.g. more than 30) that the commits 
get 'collapsed' to one diff per recipe/directory or per functional 
change. Collapsing it would be something like:

git diff .dev .merges | diffsplit

and then writing a simple script that cats the remaing files together 
based on directory (e.g. awk -F- {if $1==$2 then cat file to $3.diff)}. 
The diffsplit script can be found at [1].

If you have a functional change (e.g. like PR -> FILEPR) that can of 
course stay in one diff.

If review branches get organized like this we can more easily review 
them and suggest changes.

We will loose some history this way, but branches can get merged faster 
and it would stop huge piles of crap going in, which IMO outweighs the 
downsides.

Big branches CANNOT be merged without proper review, but I don't think 
anyone wants to review a few hundred commits :/

So, what do you think about this?


regards,

Koen


[1] http://www.pathname.com/~quinlan/software/diffsplit/diffsplit

Re: [RFC] collapse commits in branches to easy review

[Otavio Salvador]

Koen Kooi <k.kooi@student.utwente.nl> writes:

> Big branches CANNOT be merged without proper review, but I don't think
> anyone wants to review a few hundred commits :/
>
> So, what do you think about this?

Big branches are usually wrong. Instead of using diffsplit I'd suggest
that people provide the patches as patch series, as done in Linux
kernel, or in topic branches.

If the user/devel has a server where he/she can host the branches, a
set of small topic branches are easier to review and merge. Otherwise
git send-email does the work and could send the patch set to mailing
list.

My 2c.

-- 
        O T A V I O    S A L V A D O R
---------------------------------------------
 E-mail: otavio@debian.org      UIN: 5906116
 GNU/Linux User: 239058     GPG ID: 49A5F855
 Home Page: http://otavio.ossystems.com.br
---------------------------------------------
"Microsoft sells you Windows ... Linux gives
 you the whole house."

Re: [RFC] collapse commits in branches to easy review

[Koen Kooi]

On 17-12-08 16:25, Otavio Salvador wrote:
> Koen Kooi<k.kooi@student.utwente.nl>  writes:
>
>> Big branches CANNOT be merged without proper review, but I don't think
>> anyone wants to review a few hundred commits :/
>>
>> So, what do you think about this?
>
> Big branches are usually wrong. Instead of using diffsplit I'd suggest
> that people provide the patches as patch series, as done in Linux
> kernel, or in topic branches.

That's an implementation detail, I think we are after the same thing :)

The thing that prompted me to write is email is this:

http://cgit.openembedded.net/cgit.cgi?url=openembedded/log/&h=john_lee/openmoko-merges

I tried to review a portion of it, but it is too much and doing a time 
based review ends up in going "hmm, this is wrong" and seeing it is 
fixed a few commits later. But let's not get ahead of things, no-one has 
asked for a review of that branch yet :)

I'd love to see that branch as patch series where we will have a few 
review rounds, but I don't know if John was enough time to do something 
like that, so the diffsplit suggestion was there to save him a bootload 
of time.

> If the user/devel has a server where he/she can host the branches, a
> set of small topic branches are easier to review and merge. Otherwise
> git send-email does the work and could send the patch set to mailing
> list.

I agree on that.

regards,

Koen

Re: [RFC] collapse commits in branches to easy review

[Philip Balister]

[-- Attachment #1: Type: text/plain, Size: 1456 bytes --]

Koen Kooi wrote:
> Hi,
> 
> I want to propose that when people want to have a branch reviewed for 
> merging that has a lot of commits (e.g. more than 30) that the commits 
> get 'collapsed' to one diff per recipe/directory or per functional 
> change. Collapsing it would be something like:
> 
> git diff .dev .merges | diffsplit
> 
> and then writing a simple script that cats the remaing files together 
> based on directory (e.g. awk -F- {if $1==$2 then cat file to $3.diff)}. 
> The diffsplit script can be found at [1].
> 
> If you have a functional change (e.g. like PR -> FILEPR) that can of 
> course stay in one diff.
> 
> If review branches get organized like this we can more easily review 
> them and suggest changes.
> 
> We will loose some history this way, but branches can get merged faster 
> and it would stop huge piles of crap going in, which IMO outweighs the 
> downsides.
> 
> Big branches CANNOT be merged without proper review, but I don't think 
> anyone wants to review a few hundred commits :/
> 
> So, what do you think about this?

Basically, I also agree with Koen. Sadly, my git skillz are not good 
enough to understand how much of a burden this is on people. I've been 
saving this article which seems to cover the topic also:

http://www.andrewmoore.com/public/index.php/My_git_workflow

I promise to try and do enough work to have to collapse some patches for 
merging :)

Philip

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/x-pkcs7-signature, Size: 3303 bytes --]