Manually parse arguments in submit_try

Manually parse arguments in submit_try

Argparse not supported with Python < 2.7.
Committed: https://code.google.com/p/skia/source/detail?r=7403

[rmistry, 2013-01-25 19:37:01]

LGTM

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try
File tools/submit_try (right):

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try#newcode151
tools/submit_try:151: print 'submit_try: Available builders:\n  %s' % '\n 
'.join(trybots)
Doesn't '\n'.join(trybots) work here as well? is there a benefit of the spaces?

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try#newcode171
tools/submit_try:171: collected_args['revision'] = argv.pop(0)
Since 'revision' and 'changelist' are used in multiple places we should move
them to top level constants?

[EricB, 2013-01-25 19:41:12]

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try
File tools/submit_try (right):

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try#newcode151
tools/submit_try:151: print 'submit_try: Available builders:\n  %s' % '\n 
'.join(trybots)
On 2013/01/25 19:37:01, rmistry wrote:
> Doesn't '\n'.join(trybots) work here as well? is there a benefit of the
spaces?

I just think it looks prettier to have the long list indented.  It makes it
distinct from the other lines in the terminal.

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try#newcode171
tools/submit_try:171: collected_args['revision'] = argv.pop(0)
On 2013/01/25 19:37:01, rmistry wrote:
> Since 'revision' and 'changelist' are used in multiple places we should move
> them to top level constants?

I can do that.  Alternatively, I can do what argparse does and use an object
instead.

[rmistry, 2013-01-25 19:43:35]

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try
File tools/submit_try (right):

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try#newcode151
tools/submit_try:151: print 'submit_try: Available builders:\n  %s' % '\n 
'.join(trybots)
On 2013/01/25 19:41:12, EricB wrote:
> On 2013/01/25 19:37:01, rmistry wrote:
> > Doesn't '\n'.join(trybots) work here as well? is there a benefit of the
> spaces?
> 
> I just think it looks prettier to have the long list indented.  It makes it
> distinct from the other lines in the terminal.

Yes makes sense.

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try#newcode171
tools/submit_try:171: collected_args['revision'] = argv.pop(0)
On 2013/01/25 19:41:12, EricB wrote:
> On 2013/01/25 19:37:01, rmistry wrote:
> > Since 'revision' and 'changelist' are used in multiple places we should move
> > them to top level constants?
> 
> I can do that.  Alternatively, I can do what argparse does and use an object
> instead.

Top level constants should suffice, not sure if using an object will be any
better, but upto you.

[EricB, 2013-01-25 20:01:03]

Uploaded patch set 4.

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try
File tools/submit_try (right):

https://codereview.appspot.com/7206054/diff/2001/tools/submit_try#newcode171
tools/submit_try:171: collected_args['revision'] = argv.pop(0)
On 2013/01/25 19:43:35, rmistry wrote:
> On 2013/01/25 19:41:12, EricB wrote:
> > On 2013/01/25 19:37:01, rmistry wrote:
> > > Since 'revision' and 'changelist' are used in multiple places we should
move
> > > them to top level constants?
> > 
> > I can do that.  Alternatively, I can do what argparse does and use an object
> > instead.
> 
> Top level constants should suffice, not sure if using an object will be any
> better, but upto you.

I used an object.  I think it's a little cleaner this way.

[rmistry, 2013-01-25 20:10:30]

https://codereview.appspot.com/7206054/diff/2002/tools/submit_try
File tools/submit_try (right):

https://codereview.appspot.com/7206054/diff/2002/tools/submit_try#newcode130
tools/submit_try:130: class CollectedArgs(object):
This pattern is not used in python because you could do something like this:

class CollectedArgs(object):
  pass

collected_args = CollectedArgs()
collected_args.something = 'something'

The prefer pattern AFAIK is to use

class C(object):
    def __init__(self):
        self._x = None

    @property
    def x(self):
        """I'm the 'x' property."""
        return self._x

    @x.setter
    def x(self, value):
        self._x = value

[EricB, 2013-01-25 20:33:17]

Patch set 6.

https://codereview.appspot.com/7206054/diff/2002/tools/submit_try
File tools/submit_try (right):

https://codereview.appspot.com/7206054/diff/2002/tools/submit_try#newcode130
tools/submit_try:130: class CollectedArgs(object):
On 2013/01/25 20:10:30, rmistry wrote:
> This pattern is not used in python because you could do something like this:
> 
> class CollectedArgs(object):
>   pass
> 
> collected_args = CollectedArgs()
> collected_args.something = 'something'
> 
> The prefer pattern AFAIK is to use
> 
> class C(object):
>     def __init__(self):
>         self._x = None
> 
>     @property
>     def x(self):
>         """I'm the 'x' property."""
>         return self._x
> 
>     @x.setter
>     def x(self, value):
>         self._x = value

Used the latter pattern but made it immutable.

[rmistry, 2013-01-25 20:44:47]

LGTM

https://codereview.appspot.com/7206054/diff/12001/tools/submit_try
File tools/submit_try (right):

https://codereview.appspot.com/7206054/diff/12001/tools/submit_try#newcode220
tools/submit_try:220: botlist = ','.join(['%s%s' % (bot, TRYBOT_SUFFIX) for bot
in args.bots()])
optional: if you use the @property decorator you can go back to using args.bots
instead of args.bots() and similarly args.changelist and args.revision.

[EricB, 2013-01-25 20:55:42]

Committed as r7403.

https://codereview.appspot.com/7206054/diff/12001/tools/submit_try
File tools/submit_try (right):

https://codereview.appspot.com/7206054/diff/12001/tools/submit_try#newcode220
tools/submit_try:220: botlist = ','.join(['%s%s' % (bot, TRYBOT_SUFFIX) for bot
in args.bots()])
On 2013/01/25 20:44:47, rmistry wrote:
> optional: if you use the @property decorator you can go back to using
args.bots
> instead of args.bots() and similarly args.changelist and args.revision.

Done.