BIND 10 #1541: script for listing obsolete (fully merged) branches

[BIND 10 Development]

#1541: script for listing obsolete (fully merged) branches
-------------------------------------+-------------------------------------
                   Reporter:  tomek  |                 Owner:  UnAssigned
                       Type:         |                Status:  reviewing
  enhancement                        |             Milestone:
                   Priority:  low    |  Sprint-20120320
                  Component:  build  |            Resolution:
  system                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  Core
            Defect Severity:  N/A    |  Estimated Difficulty:  0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Overall, it seems to do what it intends to do.  I see some minor
 points (mainly style related ones), but excessive purism is probably
 not very productive especially for this type of helper tools.  I'll
 make some comments, but let's not spend too much time on it.  Maybe
 just after one more round of cleanup we can simply merge it and close
 the ticket.

 - for parsing options, I'd suggesting using the OptionParser module.
   see, for example, bind10_src.py.in how to use it.

 - this doesn't seem to be very clean:
 {{{#!python
 #   Uncomment this to print out also merged branches
 #    branch_print(branch_list, False, True, False)
 }}}
   I'd control the behavior via a command line option, especially when
   we already have some options.

 - I'd like to see some more comments about what each function is
   expected to do, and some more inline comments.  There's no single
   right answer this request, but for example, calling a subprocess
   would generally need an explanation..  The intent of filtering some
   outputs in branch_list_get is not very obvious (to me).

 - Some style(-like) matters:

   - "Branch" doesn't seem to have to be a class.  A tuple would be
     sufficient, although this may be a matter of taste
   - I'd add a space between '=' here (at least the spacing policy
     isn't consistent):
 {{{#!python
     MERGED=1
     NOTMERGED=2
 }}}
   - For "not assigned" type of placeholder value, we normally use
     'None':
 {{{#!python
     name = ""
     status = NOTMERGED
     last_commit = ""
 }}}
   - we don't need a semilcolon at EOL (I removed them)
   - we don't need parentheses for 'if' conditions, e.g:
 {{{#!python
         if (len(diff) == 0):
 }}}
     same for 'return'.
   - this should probably be just removed:
 {{{#!python
 #        return (out)
 }}}

-- 
Ticket URL: <http://bind10.isc.org/ticket/1541#comment:5>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development