[IPython-dev] branch management getting better...

Ondrej Certik ondrej@certik...
Wed Sep 3 17:23:19 CDT 2008


On Wed, Sep 3, 2008 at 11:08 PM, Brian Granger <ellisonbg.net@gmail.com> wrote:
>> Let's say you have 30 commits in the branch and let's say that it
>> doesn't pass the review, because it needs improving. My question is,
>> what will you do? Will you commit the improvement over your 30 commits
>> (e.g. just adding more commits to the branch, until all the code is ok
>> to go in), or will you try to fix some of the commits in the branch,
>> so that actually all of the commits individually are ok? (I know you
>> cannot just push there "individual commits", because they are related
>> with the rest.)
>
> Ahh, now I understand.  Our approach is that you create new commits
> that fix the problems in the old commits.  There are a couple of
> reasons for this:
>
> 1.  Commits serve as a full record of all the code that gets written -
> good and bad.  If you are always completely covering up bad code, it
> an hurt you.  Let's say there is a really subtle bug in a commit and
> you spend a long time tracking it down.  You put in a test case and
> all seems OK.  But if you erase the original commit, there is less of
> a record about the bug that may help you or others in the future.  I
> think it is much better to 1) have a commit that has the bug and 2)
> have another commit that fixes the bug with a full description of the
> bug.
>
> 2.  Sometimes code review requires making changes to code that spans
> multiple initial commits.  At this point, I think it just gets
> difficult to have a development model where each individual commit is
> OK by itself.  Our development style to just too non-linear and
> convoluted for this to be practical.  I suppose your style might fit
> this model better - don't know though.
>
>> I don't know if I made it clear now. My question is if you try to make
>> each change (=each commit) ok to go in, or if you just try for each
>> new bunch of commits (=branch) to be ok to go in (i.e. some commits
>> can be crap, if they are fixed by later commits in the same branch).
>
> We have a development model that allows for the following:
>
> crap + crap + crap + fix_crap + tests = beautiful code merged into trunk
>
> This seems more realistic than:
>
> beautiful_code + beautiful_code + beautiful_code + tests = beautiful_code
>
> Maybe once I become perfect, the second would work though ;-)

I see, thanks for the explanation, that's what I was interested in,
everything is now much clearer to me. Still couple points though:

I don't understand why you need to be perfect to fix crap ->
beautiful_code. In my experience, I always write:

crap+crap+crap+tests

and in the process of review, it becomes:

beautiful_code + beautiful_code + beautiful_code + tests

while in your case, it becomes:

crap+crap+crap+tests+fix_crap

E.g. the only difference is that in my case I distribute the
"fix_crap" commit into the commits that are crap.
I guess it depends on the "fix_crap". If it's just a simple fix, it's ok.

I am still trying to find the best workflow, but so far I like this: I
commit very often and prepare something in my local branch. Then I
rebase it in a couple of well documented nice commits, put it in some
public branch (so that people can pull it) and ask for a review. Then,
if I did a good job and the "fix_crap" commit (there is almost always
some) is simple, I completely agree with you it should be committed
after it. But if I did a bad job and the whole branch needs to be
revised, or just more deep changes are needed, it's imho better to
rework it. E.g. what happens in your case, if the branch is "not ok to
go in", but the fix is not just some easy "fix_crap", but some
fundamental problem -- you just start another branch and use the good
ideas (patches) from the broken one?

Ondrej


More information about the IPython-dev mailing list