Code review comment for lp:~jkakar/bzr/custom-project-name

> There are a few stylistic things that we generally prefer. For example:
> +def set_program_info(info):
>
> ^- two blank lines before 'set'

Thanks, fixed.

> + self.assertEquals(info.name, 'name')
> + self.assertEquals(info.version, 'version')
>
> ^- Generally we use "assertEqual()" and put the expected form on the left.
>
> self.assertEqual('name', info.name)
> self.assertEqual('version', info.version)

Thanks, fixed.

> ^- I understand why you want to do it. However, it feels harder to write
> and maintain our own bzrlib code to have to put %(program-name)
> everywhere. And it is somewhat likely to regress for you, since we could
> easily add another help line anywhere in the program and not realize the
> need to %(program-name)s escape it.
>
> My *personal* feeling is that re-using the Command infrastructure is
> encouraged and patches to make that easier are welcome.
>
> Reusing an individual cmd_xxx class is allowed, but it doesn't make a
> lot of sense to make special provisions for. Certainly you could just
> subclass and override the docstring:
>
> class cmd_alias(bzrlib.builtins.cmd_alias):
> """New doc string with my program name."""

Yeah, this change isn't 100% necessary and I can totally understand
why you're unhappy with it. What I do right now in Commandant is
s/bzr/my-program-name/ before using content produced by
Command.get_help_text and that works fine.

> You'll likely need to do the inheritance anyway, to inject whatever
> changes you need to do to get the aliases to show up in your config
> file, rather than in ours.

What I'm hoping to do is make the configuration system in bzrlib
pluggable. I'd prefer not to reimplement cmd_alias if possible.
All I really need to do is for bzrlib.config.config_filename to
return a path to my config file instead of ~/.bazaar/bazaar.conf and
it'll just work.

> So *I* personally don't like the change from "bzr => %(program-name)s"
> in the help text. I'm not going to veto it, but I'd at least like us to
> discuss it before we merge it.

I don't have a strong opinion about the change. If it feels ugly
I'd be happy to back it out. We can always introduce it again in
the future if we decide it makes sense to have.

Thanks for the review!