[rfc][patch]

[John Arbash Meinel]

Erik Bågfors wrote:
> 2006/1/2, John Arbash Meinel <john at arbash-meinel.com>:

...

>>>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 use purely tabs in my own source code, but I follow bzr's guidelines
in its source. So I use a different setup.
In ~/.vim/filetype.vim I have the lines:
au BufNewFile,BufRead **/bzr/**/*.py setlocal ts=8 sts=4 sw=4 et ai si

So if I'm editing a python file underneath a 'bzr' directory, it sets
everything correctly.

> 
>>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.

Like I said, it is a little bit debatable. But it seems like if we are
going to pick a standard, it would be (expected, actual).
I prefer it because it is easier to read the expected from:

Assertion "Something that is pretty long is not equal to" != "Something
that is pretty long is equal to"

> 
> 
>>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
> 

I don't get any errors for your branch and ./bzr selftest.

So now, after all this back and forth. I'm finally a full +1 on it. We
just need to get another.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060103/31046ada/attachment.pgp