[MERGE] bzr branch description support

Aaron Bentley aaron.bentley at utoronto.ca
Mon Sep 17 19:37:34 BST 2007


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

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.

bb:resubmit

> @@ -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')[0]
>      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
pair.

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'.')[0]
> +        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.

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

iD8DBQFG7slu0F+nu1YWqI0RAsZbAJ9SHltQG/S56wmBnxEeMlsIhnaDZgCfXfkc
z4tYo1d+12LagGS4ssd+Pos=
=CI6E
-----END PGP SIGNATURE-----



More information about the bazaar mailing list