On Mon, Sep 06, 2010 at 10:18:00PM -0400, Chris Ball wrote:
> > This should probably be
> > bugs = [bug for bug in get_blocked_by(self.bd, targetbug) if not bug.active]
> 
> Thanks, fixed.  ("if bug.active", rather than "if not")

Oops ;).  Thanks.

On Mon, Sep 06, 2010 at 10:23:55PM -0400, Chris Ball wrote:
>    > There should also be some way to remove targets.
> 
> Thanks, fixed as:
>         if target:
>             current_target = bug_target(self.bd, bug)
>             if current_target:
>                 remove_target(self.bd, bug)
>                 if target != "None":
>                     add_target(self.bd, bug, target)
>             else:
>                 add_target(self.bd, bug, target)

Looks good.

> I was mildly surprised that bug_target() throws an exception on a bug
> with more than one target -- is that a limitation of targets, or just
> of bug_target()?  If the former, we should probably make add_target()
> refuse to add a target to a bug that already has one..

You're right, you should probably be able to add multiple targets.  I
put in the restriction because I always envisioned targets in series:

  target: 1.0.beta
    bug: improve whatsit interface
    target: 1.0.alpha
      bug: basic whatsit functionality

where `1.0.beta' would block on `basic whatsit functionality`
indirectly via `1.0.alpha`, and probably also via `improve whatsit
interface' which would have it as a regular (non-target) dependency.

You would, however, need multiple targets if you had parallel targets:

  target: 1.0.beta
    target: 1.0.ui
      bug: basic whatsit functionality
    target: 1.0.core
      bug: basic whatsit functionality

Since *somebody* will probably want to do this, we should relax the
restriction and allow multiple targets.  In fact, people can currently
add multiple targets via the `be depend`, bypassing any restrictions
imposed by `be target`.

> Will push this patchset along with a merge of your tree to gitorious,
> unless you have any more improvements on this patch.  Thanks for the
> review!

Push away, unless you want to add a multiple-targets patch first.
Otherwise, I can work it in after I've pulled your new stuff.

-- 
This email may be signed or encrypted with GPG (http://www.gnupg.org).
The GPG signature (if present) will be attached as 'signature.asc'.
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

My public key is at http://www.physics.drexel.edu/~wking/pubkey.txt

Attachment: pgpoVr1qck0jB.pgp
Description: PGP signature

_______________________________________________
Be-devel mailing list
[email protected]
http://void.printf.net/cgi-bin/mailman/listinfo/be-devel

Reply via email to