[MERGE] bzr branch description support
aaron.bentley at utoronto.ca
Mon Sep 17 19:37:34 BST 2007
-----BEGIN PGP SIGNED MESSAGE-----
Ali Sabil wrote:
> I think that being able to attach a description to a branch will improve
> the distributed part of bzr, so I went down and added description
> support, it is implemented in the same way as branch nickname, so the
> branch description is not propagated. I hope it would be possible to
> improve this :)
The idea and approach seem fine to me (after all, I suggested the
approach). There are some problems in the details.
> @@ -0,0 +1,37 @@
> +# Copyright (C) 2005, 2006 Canonical Ltd
^^^ This should be 2007 for a new file.
> +class TestDescription(ExternalBase):
> + def test_description_command(self):
> + """bzr description for viewing, setting description"""
> + self.make_branch_and_tree('me.dev')
> + os.chdir('me.dev')
^^^ commands that use a branch should provide a way to specify that
branch. Either it should be an argument or an option (like pull's -d or
send's -f). chdir isn't enough, because it doesn't provide a way to
refer to remote branches.
> + description = self.run_bzr('description')
> nick = property(_get_nick, _set_nick)
> + def _get_description(self):
> + return self.get_config().get_description()
> + def _set_description(self, description):
> + self.get_config().set_user_option('description', description, warn_masked=True)
> + description = property(_get_description, _set_description)
The fact that nick uses a property is considered a mistake, because
retrieving the nick can be expensive over a remote link. Expensive
operations shouldn't look like attribute access, so please use a set/get
PEP8 says this needs another newline between cmd_nick and cmd_description
> +class cmd_description(Command):
> + """Print or set the branch description.
> + If unset, the tree root directory name is used as the description
> + To print the current description, execute with no argument.
> + """
> + _see_also = ['info']
^^^ This isn't appropriate, since there's nothing in "info" about
descriptions. It would be nice to include the description in the info,
though, and then this would make sense.
> + takes_args = ['description?']
> + def run(self, description=None):
> + branch = Branch.open_containing(u'.')
> + if description is None:
> + self.printme(branch)
> + else:
> + branch.description = description
> + @display_command
> + def printme(self, branch):
> + print branch.description
> class cmd_selftest(Command):
> """Run internal test suite.
Again, PEP8 demands another newline.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar