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

[John Arbash Meinel]

James Westby wrote:
> On (11/04/07 14:43), Jonathan Lange wrote:
>> Adds a '--fixes' option to commit to allow users to specify that the
>> commit fixes certain bugs. Bugfix information is stored as <bug_url>:
>> <status> entry in the properties dict.
>>
>> Based on discussions w/ lifeless and jamesh at the recent Sydney 
>> mini-sprint.
>>
> 
> Hi,
> 
> This looks good.
> 
> I have a couple of points.
> 
> Is it possible to set the default tracker for a branch? This would allow
> --fixes 12345 to do the right thing, but --fixes lp:21345 to be used if
> wanted?
> 
> Also I have added bugs.debian.org, with the tag 'deb'. I'm not sure that
> it is the correct tag, but it will do. Also b.d.o allows you to use the
> #21345 in the URL and the redirect will work, so that could be allowed
> but isn't. I have added two classes to help with adding new trackers. If
> you have a tracker where a bug_id is just appended to a base URI then
> subclass the first and add attributes for the tag and base_url. The
> second is the same but ensures the bug_ids are integers. I moved lp over
> to the second as well.
> 
> Would it be possible to take these and make it easy to use a new one
> that is simple like these. Like you do with the trac URLs, but just
> define a generic URL. I wasn't sure on the best approach, so I thought I
> would ask first.
> 
> The bundle goes on top of yours.
> 
> Did all the tests pass for you? I had a couple of problems, namely the
> Option tests, where it appears the ListOption was causing some trouble.
> 
> Thanks,
> 
> James
> 
> 
> 
> ------------------------------------------------------------------------
> 
> # Bazaar revision bundle v0.9
> #
> # message:
> #   Add a superclass for easy bug trackers. Also add bugs.debian.org as deb:
> #   
> # committer: James Westby <jw+debian at jameswestby.net>
> # date: Thu 2007-04-12 21:58:30.006999969 +0100
> 
> === modified file bzrlib/bugtracker.py
> --- bzrlib/bugtracker.py
> +++ bzrlib/bugtracker.py
> @@ -60,27 +60,58 @@
>  tracker_registry = TrackerRegistry()
>  """Registry of bug trackers."""
>  
> -
> -class LaunchpadTracker(object):
> +class SimpleBugtracker(object):
> +    """A bug tracker that where bug numbers are appended to a base URL.
> +    
> +    If you have one of these trackers then subclass this and add attributes
> +    named 'tag' and 'base_url'. The former is the tag 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.
> +    """
> +
> +    @classmethod
> +    def get(klass, tag, branch):
> +        """Returns the tracker if the tag matches. Returns None otherwise."""
> +        if tag != klass.tag:
> +            return None
> +        return klass()
> +
> +    def get_bug_url(self, bug_id):
> +        """Return the URL for bug_id."""
> +        self.check_bug_id(bug_id)
> +        return self.base_url + bug_id
> +
> +    def check_bug_id(self, bug_id):
> +        """Check that the bug_id is valid."""
> +        pass
> +
> +
> +class SimpleIntegerBugtracker(SimpleBugtracker):
> +    """A SimpleBugtracker where the bug ids must be integers"""
> +
> +    def check_bug_id(self, bug_id):
> +        try:
> +            int(bug_id)
> +        except ValueError:
> +            raise errors.MalformedBugIdentifier(bug_id, "Must be an integer")

To start with, I think this is a nice refactoring. And should make
implementing new bug tracker support easier.

If you are going to introduce SimpleBugTracker and
SimpleIntegerBugTracker, then then tests on LaunchpadTracker for these
functions should be moved to a test on them specifically. (Or possibly
just copied there).

Certainly those classes should be directly tested. Especially since it
is intended that people will use them as a base class for their bug
trackers.

I would also probably say that the base implementation should have:

  tag = None
  base_url = None

To make it clear that the base class expects these parameters to exist.
You might also want to add:

:cvar tag: Info about tag
:cvar base_url: Info about base_url

to document the class variables tag and base_url in the SimpleBugTracker.


John
=:->