[PATCH] modify: new option --dependson

[Don Zickus]

A lot of our bugs are collected into higher level bugzillas.  Having the
ability to add, remove, overwrite the --dependson field becomes necessary.

All the backend logic exists, just implement the front end logic.

The patch is pretty straightforward except for the duplicate code to handle
comma seperated lists.

The expected command is

bugzilla modify --dependons=123456,456789 987654

This would add bugs 123456 and 456789 to the depends on field for bz987654

Signed-off-by: Don Zickus <dzickus(a)redhat.com&gt;
---
 bin/bugzilla |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/bin/bugzilla b/bin/bugzilla
index 3ec2be8..90282e8 100755
--- a/bin/bugzilla
+++ b/bin/bugzilla
@@ -279,6 +279,9 @@ def setup_action_parser(action):
                      '(Use a new option for each flag)')
         p.add_option('--cc', action='append',
                 help='Add an email to the cc list')
+        p.add_option('--dependson', metavar='BUGID[, BUGID, ...]',
+                help=('Alter depends_on list.  BUGID appends, '
+                     '-BUGID removes, =BUGID overwrites'))
         p.add_option('-F', '--fixed_in', metavar="VERSION",
                 help='"Fixed in version" field')
         p.add_option("", "--whiteboard", metavar="TEXT",
@@ -732,12 +735,32 @@ def _do_modify(bz, opt, args):
             add_val = val
         return add_val, rm_val, set_val
 
+    def parse_triset_list(val):
+        val = val or ""
+        add_val = None
+        rm_val = None
+        set_val = None
+
+        if val.startswith("+"):
+            add_val = val[1:].split(",")
+        elif val.startswith("-"):
+            rm_val = val[1:].split(",")
+        elif val.startswith("="):
+            set_val = val[1:].split(",")
+        else:
+            add_val = val.split(",")
+        return add_val, rm_val, set_val
+
     add_wb, rm_wb, set_wb = parse_triset(opt.whiteboard)
+    add_deps, rm_deps, set_deps = parse_triset_list(opt.dependson)
 
     update = bz.build_update(
         comment=opt.comment or None,
         comment_private=opt.private or None,
         cc_add=opt.cc and opt.cc[:] or None,
+        depends_on_add=add_deps or None,
+        depends_on_remove=rm_deps or None,
+        depends_on_set=set_deps or None,
         qa_contact=opt.qa_contact or None,
         assigned_to=opt.assignee or None,
         status=status,
@@ -966,7 +989,7 @@ def main(bzinstance=None):
         if not (opt.status or opt.close or
                 opt.assignee or opt.qa_contact or
                 opt.flag or opt.cc or opt.comment or
-                opt.fixed_in or opt.whiteboard):
+                opt.fixed_in or opt.whiteboard or opt.dependson):
             parser.error("'modify' command requires additional
arguments")
 
         if not args:
-- 
1.7.1

[Cole Robinson]

On 03/11/2013 05:25 PM, Don Zickus wrote:
>  A lot of our bugs are collected into higher level bugzillas.  Having
> the
>  ability to add, remove, overwrite the --dependson field becomes necessary.
>  
>  All the backend logic exists, just implement the front end logic.
>  
>  The patch is pretty straightforward except for the duplicate code to handle
>  comma seperated lists.
>  
>  The expected command is
>  
>  bugzilla modify --dependons=123456,456789 987654
>  
>  This would add bugs 123456 and 456789 to the depends on field for bz987654
>   
Sounds good to me.

>   
>  +    def parse_triset_list(val):
>  +        val = val or ""
>  +        add_val = None
>  +        rm_val = None
>  +        set_val = None
>  +
>  +        if val.startswith("+"):
>  +            add_val = val[1:].split(",")
>  +        elif val.startswith("-"):
>  +            rm_val = val[1:].split(",")
>  +        elif val.startswith("="):
>  +            set_val = val[1:].split(",")
>  +        else:
>  +            add_val = val.split(",")
>  +        return add_val, rm_val, set_val
>  + 
Rather than duplicate the parse_triset() logic, just call it and .split(",")
each returned value.

Patch looks good otherwise, after that change I'll extend the test suite to
cover it as well.

Thanks,
Cole

[Don Zickus]

On Mon, Mar 11, 2013 at 05:33:19PM -0400, Cole Robinson wrote:
>  On 03/11/2013 05:25 PM, Don Zickus wrote:
>  > A lot of our bugs are collected into higher level bugzillas.  Having the
>  > ability to add, remove, overwrite the --dependson field becomes necessary.
>  > 
>  > All the backend logic exists, just implement the front end logic.
>  > 
>  > The patch is pretty straightforward except for the duplicate code to handle
>  > comma seperated lists.
>  > 
>  > The expected command is
>  > 
>  > bugzilla modify --dependons=123456,456789 987654
>  > 
>  > This would add bugs 123456 and 456789 to the depends on field for bz987654
>  > 
>  
>  Sounds good to me.
>  
>  >  
>  > +    def parse_triset_list(val):
>  > +        val = val or ""
>  > +        add_val = None
>  > +        rm_val = None
>  > +        set_val = None
>  > +
>  > +        if val.startswith("+"):
>  > +            add_val = val[1:].split(",")
>  > +        elif val.startswith("-"):
>  > +            rm_val = val[1:].split(",")
>  > +        elif val.startswith("="):
>  > +            set_val = val[1:].split(",")
>  > +        else:
>  > +            add_val = val.split(",")
>  > +        return add_val, rm_val, set_val
>  > +
>  
>  Rather than duplicate the parse_triset() logic, just call it and .split(",")
>  each returned value. 
I thought about that, then wondered what happens if the whiteboard has a
',' in it?  It might get accidentally stripped.

Actually, just tried that and noticed that in the bottom of
bin/bugzilla::_do_modify is this snippet:

    for bug in bz.getbugs(bugid_list):
        if add_wb:
            bug.addtag(add_wb)
        if rm_wb:
            bug.deltag(rm_wb)

Which requires the *_wb stuff to be a string and not an array. :-(

Not sure how much of the array you want to propogate to the backend.

Thoughts?

Cheers,
Don

[Cole Robinson]

On 03/12/2013 09:40 AM, Don Zickus wrote:
>  On Mon, Mar 11, 2013 at 05:33:19PM -0400, Cole Robinson wrote:
> > On 03/11/2013 05:25 PM, Don Zickus wrote:
> >> A lot of our bugs are collected into higher level bugzillas.  Having the
> >> ability to add, remove, overwrite the --dependson field becomes necessary.
> >>
> >> All the backend logic exists, just implement the front end logic.
> >>
> >> The patch is pretty straightforward except for the duplicate code to handle
> >> comma seperated lists.
> >>
> >> The expected command is
> >>
> >> bugzilla modify --dependons=123456,456789 987654
> >>
> >> This would add bugs 123456 and 456789 to the depends on field for bz987654
> >>
> >
> > Sounds good to me.
> >
> >>  
> >> +    def parse_triset_list(val):
> >> +        val = val or ""
> >> +        add_val = None
> >> +        rm_val = None
> >> +        set_val = None
> >> +
> >> +        if val.startswith("+"):
> >> +            add_val = val[1:].split(",")
> >> +        elif val.startswith("-"):
> >> +            rm_val = val[1:].split(",")
> >> +        elif val.startswith("="):
> >> +            set_val = val[1:].split(",")
> >> +        else:
> >> +            add_val = val.split(",")
> >> +        return add_val, rm_val, set_val
> >> +
> >
> > Rather than duplicate the parse_triset() logic, just call it and
> .split(",")
> > each returned value.
>  
>  I thought about that, then wondered what happens if the whiteboard has a
>  ',' in it?  It might get accidentally stripped.
>  
>  Actually, just tried that and noticed that in the bottom of
>  bin/bugzilla::_do_modify is this snippet:
>  
>      for bug in bz.getbugs(bugid_list):
>          if add_wb:
>              bug.addtag(add_wb)
>          if rm_wb:
>              bug.deltag(rm_wb)
>  
>  Which requires the *_wb stuff to be a string and not an array. :-(
>  
>  Not sure how much of the array you want to propogate to the backend.
>  
>  Thoughts? 
I meant

def parse_triset_list(val):
   add_val, rm_val, set_val = parse_triset(val)
   return add_val.split(","), rm_val.split(","),
set_val.split(",")

And leave the whiteboard stuff untouched.

Thanks,
Cole

[Don Zickus]

On Tue, Mar 12, 2013 at 09:55:03AM -0400, Cole Robinson wrote:
>  On 03/12/2013 09:40 AM, Don Zickus wrote:
>  > On Mon, Mar 11, 2013 at 05:33:19PM -0400, Cole Robinson wrote:
>  >> On 03/11/2013 05:25 PM, Don Zickus wrote:
>  >>> A lot of our bugs are collected into higher level bugzillas.  Having the
>  >>> ability to add, remove, overwrite the --dependson field becomes necessary.
>  >>>
>  >>> All the backend logic exists, just implement the front end logic.
>  >>>
>  >>> The patch is pretty straightforward except for the duplicate code to handle
>  >>> comma seperated lists.
>  >>>
>  >>> The expected command is
>  >>>
>  >>> bugzilla modify --dependons=123456,456789 987654
>  >>>
>  >>> This would add bugs 123456 and 456789 to the depends on field for bz987654
>  >>>
>  >>
>  >> Sounds good to me.
>  >>
>  >>>  
>  >>> +    def parse_triset_list(val):
>  >>> +        val = val or ""
>  >>> +        add_val = None
>  >>> +        rm_val = None
>  >>> +        set_val = None
>  >>> +
>  >>> +        if val.startswith("+"):
>  >>> +            add_val = val[1:].split(",")
>  >>> +        elif val.startswith("-"):
>  >>> +            rm_val = val[1:].split(",")
>  >>> +        elif val.startswith("="):
>  >>> +            set_val = val[1:].split(",")
>  >>> +        else:
>  >>> +            add_val = val.split(",")
>  >>> +        return add_val, rm_val, set_val
>  >>> +
>  >>
>  >> Rather than duplicate the parse_triset() logic, just call it and
> .split(",")
>  >> each returned value.
>  > 
>  > I thought about that, then wondered what happens if the whiteboard has a
>  > ',' in it?  It might get accidentally stripped.
>  > 
>  > Actually, just tried that and noticed that in the bottom of
>  > bin/bugzilla::_do_modify is this snippet:
>  > 
>  >     for bug in bz.getbugs(bugid_list):
>  >         if add_wb:
>  >             bug.addtag(add_wb)
>  >         if rm_wb:
>  >             bug.deltag(rm_wb)
>  > 
>  > Which requires the *_wb stuff to be a string and not an array. :-(
>  > 
>  > Not sure how much of the array you want to propogate to the backend.
>  > 
>  > Thoughts?
>  
>  I meant
>  
>  def parse_triset_list(val):
>     add_val, rm_val, set_val = parse_triset(val)
>     return add_val.split(","), rm_val.split(","),
> set_val.split(",")
>  
>  And leave the whiteboard stuff untouched. 
Ah, sorry for being thick.  However, two of the three vars are usually
None, so the code isn't as simple as above.  My python isn't that strong so
there might be a simpler way to do the None check, but here is the newer
version of the patch.

Cheers,
Don

-----------------------------8<-----------------------
From: Don Zickus <dzickus(a)redhat.com&gt;
Date: Mon, 11 Mar 2013 17:15:22 -0400
Subject: [PATCH] modify: new option --dependson

A lot of our bugs are collected into higher level bugzillas.  Having the
ability to add, remove, overwrite the --dependson field becomes necessary.

All the backend logic exists, just implement the front end logic.

The patch is pretty straightforward except for the duplicate code to handle
comma seperated lists.

The expected command is

bugzilla modify --dependons=123456,456789 987654

This would add bugs 123456 and 456789 to the depends on field for bz987654

V2: consolidate parse_triset_list to re-use parse_triset

Signed-off-by: Don Zickus <dzickus(a)redhat.com&gt;
---
 bin/bugzilla |   19 ++++++++++++++++++-
 1 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/bin/bugzilla b/bin/bugzilla
index 3ec2be8..881c44e 100755
--- a/bin/bugzilla
+++ b/bin/bugzilla
@@ -279,6 +279,9 @@ def setup_action_parser(action):
                      '(Use a new option for each flag)')
         p.add_option('--cc', action='append',
                 help='Add an email to the cc list')
+        p.add_option('--dependson', metavar='BUGID[, BUGID, ...]',
+                help=('Alter depends_on list.  BUGID appends, '
+                     '-BUGID removes, =BUGID overwrites'))
         p.add_option('-F', '--fixed_in', metavar="VERSION",
                 help='"Fixed in version" field')
         p.add_option("", "--whiteboard", metavar="TEXT",
@@ -732,12 +735,26 @@ def _do_modify(bz, opt, args):
             add_val = val
         return add_val, rm_val, set_val
 
+    def parse_triset_list(val):
+        add_val, rm_val, set_val = parse_triset(val)
+        if add_val:
+            add_val = add_val.split(",")
+        if rm_val:
+            rm_val = rm_val.split(",")
+        if set_val:
+            set_val = set_val.split(",")
+        return add_val, rm_val, set_val
+
     add_wb, rm_wb, set_wb = parse_triset(opt.whiteboard)
+    add_deps, rm_deps, set_deps = parse_triset_list(opt.dependson)
 
     update = bz.build_update(
         comment=opt.comment or None,
         comment_private=opt.private or None,
         cc_add=opt.cc and opt.cc[:] or None,
+        depends_on_add=add_deps or None,
+        depends_on_remove=rm_deps or None,
+        depends_on_set=set_deps or None,
         qa_contact=opt.qa_contact or None,
         assigned_to=opt.assignee or None,
         status=status,
@@ -966,7 +983,7 @@ def main(bzinstance=None):
         if not (opt.status or opt.close or
                 opt.assignee or opt.qa_contact or
                 opt.flag or opt.cc or opt.comment or
-                opt.fixed_in or opt.whiteboard):
+                opt.fixed_in or opt.whiteboard or opt.dependson):
             parser.error("'modify' command requires additional
arguments")
 
         if not args:
-- 
1.7.1

[Cole Robinson]

On 03/12/2013 11:16 AM, Don Zickus wrote:
>  On Tue, Mar 12, 2013 at 09:55:03AM -0400, Cole Robinson wrote:
> > On 03/12/2013 09:40 AM, Don Zickus wrote:
> >> On Mon, Mar 11, 2013 at 05:33:19PM -0400, Cole Robinson wrote:
> >>> On 03/11/2013 05:25 PM, Don Zickus wrote:
> >>>> A lot of our bugs are collected into higher level bugzillas.  Having the
> >>>> ability to add, remove, overwrite the --dependson field becomes
> necessary.
> >>>>
> >>>> All the backend logic exists, just implement the front end logic.
> >>>>
> >>>> The patch is pretty straightforward except for the duplicate code to
> handle
> >>>> comma seperated lists.
> >>>>
> >>>> The expected command is
> >>>>
> >>>> bugzilla modify --dependons=123456,456789 987654
> >>>>
> >>>> This would add bugs 123456 and 456789 to the depends on field for
> bz987654
> >>>>
> >>>
> >>> Sounds good to me.
> >>>
> >>>>  
> >>>> +    def parse_triset_list(val):
> >>>> +        val = val or ""
> >>>> +        add_val = None
> >>>> +        rm_val = None
> >>>> +        set_val = None
> >>>> +
> >>>> +        if val.startswith("+"):
> >>>> +            add_val = val[1:].split(",")
> >>>> +        elif val.startswith("-"):
> >>>> +            rm_val = val[1:].split(",")
> >>>> +        elif val.startswith("="):
> >>>> +            set_val = val[1:].split(",")
> >>>> +        else:
> >>>> +            add_val = val.split(",")
> >>>> +        return add_val, rm_val, set_val
> >>>> +
> >>>
> >>> Rather than duplicate the parse_triset() logic, just call it and
> .split(",")
> >>> each returned value.
> >>
> >> I thought about that, then wondered what happens if the whiteboard has a
> >> ',' in it?  It might get accidentally stripped.
> >>
> >> Actually, just tried that and noticed that in the bottom of
> >> bin/bugzilla::_do_modify is this snippet:
> >>
> >>     for bug in bz.getbugs(bugid_list):
> >>         if add_wb:
> >>             bug.addtag(add_wb)
> >>         if rm_wb:
> >>             bug.deltag(rm_wb)
> >>
> >> Which requires the *_wb stuff to be a string and not an array. :-(
> >>
> >> Not sure how much of the array you want to propogate to the backend.
> >>
> >> Thoughts?
> >
> > I meant
> >
> > def parse_triset_list(val):
> >    add_val, rm_val, set_val = parse_triset(val)
> >    return add_val.split(","), rm_val.split(","),
> set_val.split(",")
> >
> > And leave the whiteboard stuff untouched.
>  
>  Ah, sorry for being thick.  However, two of the three vars are usually
>  None, so the code isn't as simple as above.  My python isn't that strong so
>  there might be a simpler way to do the None check, but here is the newer
>  version of the patch.
>   
Good point. Patch looks good, pushed now. Thanks Don!

- Cole