[MERGE] Initial support for marking bugs as fixed using bzr

[Robert Collins]

On Mon, 2007-04-23 at 10:56 +1000, Jonathan Lange wrote:
> On 4/19/07, Robert Collins <robertc at robertcollins.net> wrote:
> > On Fri, 2007-04-13 at 17:33 +1000, Jonathan Lange wrote:

> > I think referring to these as 'tags' is potentially confusing.
> Branch
> > have tags. Chatting on IRC we came up with
> > 'abbreviated_bugtracker_name'. While this is a mouthful, its clear what
> > it refers to. I'd like you to look for 'tag' throughout this file and
> > use a more precise name that isn't overloaded within bzrlib already.
> > E.g. withing the tracker classes, perhaps 'abbreviation' is a good
> > attribute name.
> >
> 
> Changed to abbreviatied_bugtracker_name and abbreviation in code and docs.

I hope thats not literal ? (tied ???)

> > As a side note, launchpad doesn't strictly exist in only one place - the
> > demo and staging servers have different databases to the production
> > server and can thus talk about different bug numbers. I'm not sure it
> > matters - but perhaps for testing it would be nice to be able to have
> > throwaway branches that you use with e.g. staging, and have the right
> > urls created.
> >
> 
> I haven't really done anything about this.

Perhaps file a bug about this?


> > I dont like that you have subclasses for Launchpad and Debian that vary
> > solely in parameters. That to me cries out for instances. This would in
> > principal allow configuring abbreviations for any tracker that we have
> > the basic facility for via branch.conf - or any other config file in the
> > configuration system.
> 
> Yeah, I don't like it either. I've changed them to instances, but not
> updated their config mechanism -- they are just registered manually.

+
+class UniqueBugTracker(object):
+    """A style of bug tracker that exists in one place only, such as Launchpad.
+
+    If you have one of these trackers then subclass this and add attributes
+    named 'abbreviation' and 'base_url'. The former is the abbreviation that
+    the user will use on the command line. The latter is the url that the bug
+    ids will be appended to.
+
+    If the bug_id must have a special form then override check_bug_id and
+    raise an exception if the bug_id is not valid.
+    """

This docstring is now wrong :). Subclassing and adding attributes is not
the right way to reuse this class. (Subclassing to define behaviour is,
instanciating to set abbreviation and url is too)



+class TracTracker(object):
+    """A Trac instance."""

and


+class BugzillaTracker(object):
+    """A Bugzilla instance."""
+

Have quite a bit of duplicate code. And more still is duplicated with 


+class UniqueIntegerBugTracker(UniqueBugTracker):
+    """A SimpleBugtracker where the bug ids must be integers"""
+
+    def check_bug_id(self, bug_id):

I'd like to see that consolidated :). (This is what I meant by saying it
would be good for code factoring).

On the docs:

+Bug Tracker Options
+===================
+
+These options can go into bazaar.conf or into a branch-specific
configuration
+section in locations.conf.

That can also go into branch.conf right ?

> Points addressed. Thanks for the really helpful review.

Cool! Glad it was helpful. We're nearly there from my point of view  :)

-Rob



-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20070423/3b8e6187/attachment.pgp