[rfc][patch]

Erik Bågfors zindar at gmail.com
Tue Jan 3 10:03:07 GMT 2006


2006/1/2, John Arbash Meinel <john at arbash-meinel.com>:
> Erik Bågfors wrote:
> > 2005/12/24, Erik Bågfors <zindar at gmail.com>:
> >
>
> ...
>
> >>Oh, that was just a mistake.
> >>
> >>Both fixed. In the same place.
> >
> >
> >
> > John, Can you take a look at this again?
>
> In bzrlib/builtins.py and blackbox/test_pull.py you have tab characters
> which need to be expanded.


Arrghhh

This is one of the reasons I do not like python. I think it's
braindead to use indentation like this. :)

Anyone here using vim that can show me their setup? If I have the
following line (S=space, T=tab)

SSSSSSfoo=5

And then go to the end of that line, and hit enter, vim helps me by
doing the following
T<cursor>

And messes everything up.

What can I do about this?

>
> I think it is better to do:
> rev_id = revision[0].in_history(br_from).rev_id
>
> _match_on() is a private interface.

Abolutely, this is the thing that I was most uncertain about.

> If we are being picky, you tests do:
> self.assertEquals(a.revno(), 4)
>
> I think we decided the prefered way is
> self.assertEquals(4, a.revno())
>
> I agree that the former feels a little bit more natural. But when
> reviewing errors, it is easier to read the correct value first. And it
> is nice if we standardize, so that when reading test failures, it is
> easier to see what is wrong.
>
> This isn't a big deal with simple numbers. It is more when you have a
> long array, which is breaking across lines, etc.

I left this, since people seams to have different views and further
down in the code I find the following

             self.assertEqual(len(rh), expected_len)

So it seams (actual,expected) is already used.

I'm fine with changing this is you prefer.

> The concept is +1, just needs a couple of refinements to be code clean.

Ok, pull from my branch again.

I now get it to pass the following
BZR_PLUGIN_PATH='' ./bzr selftest 'pull.*revision'
If I try the full selftest I get errors, but I get the exact same
errors if I do it in bzr.dev, so I don't think I'm causing it :)

Regards,
Erik




More information about the bazaar mailing list