[MERGE REVIEW] repository UI mark II

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
| Looks good at first read... modulo one thing
| I think you should capture the output and check it looks right:
|           out, err = self.run_bzr("init-repository", "a")
|           self.assertEqual("Created repository at ...", out)
|           self.assertEqual("". err)
|
|           (or whatever it should be).

Well, it's the strong, silent type, like bzr init.  But I have added a
test to ensure it prints nothing.

|>+        self.assertIs(os.path.exists("a/.bzr/repository"), True)
|>+        self.assertIs(os.path.exists("a/.bzr/branch"), False)
|>+        self.assertIs(os.path.exists("a/.bzr/checkout"), False)
|
|
| I think the following reads more clearly:
|           self.assertTrue(os.path.exists('a/.bzr/repository'))
|           self.assertFalse(os.path.exists('a/.bzr/branch'))
|           self.assertFalse(os.path.exists('a/.bzr/checkout'))

I don't want something that evaluates to True, I want something that is
True.  Conversion to bool is way too fuzzy for my taste.  I don't want
to treat 'x' as True, and I don't want to treat [] as False.

| but if the intent is to test that a shared repository is created with no
| branch or checkout, the following might be more appropriate - given we
| are not trying to test layout here

What we do care about, though, is that the repository is not at the same
location as the branch.

| rather ui operation:
|         dir = bzrlib.bzrdir.BzrDir.open('a')
|         self.assertTrue(b.open_repository().is_shared())
|         self.assertRaises(errors.NotBranchError, dir.open_branch)
|         self.assertRaises(errors.NoWorkingTree, dir.open_workingtree)

I thought these were supposed to be blackbox tests, so it seems pretty
weird using the API to test them.

| That will continue to operate correctly even if we change the disk
| layout in the future, reducing test churn.

Yes, but it will break if the API changes.  I think that's more likely
than a metadir format change, but whatever, I'll change it.

| Other than that its +1 from me. I think that init-repository as the
| command <may> need tweaking but thats up to Martin - I have no strong
| opinion.

I'm not wedded to it either, but it seemed more consistent than
'make-repository'.

I've attached a patch showing the changes I made, per your suggestions.
~ But you can also grab it from my repository if you prefer.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFEHY6u0F+nu1YWqI0RAiECAJ9O6vuO26HE09nEH3Bphot0yUmyOACdGAoh
SAmWv38OcQPn5XjY0Mreo9k=
=YqDK
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: repository-ui-4.patch
Type: text/x-patch
Size: 2449 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060319/01b26936/attachment.bin