[Findit] Make sure not use empty list or dict as default value.

[Findit] Make sure not use empty list or dict as default value.

BUG=

Committed: https://chromium.googlesource.com/infra/infra/+/d7b4ad09657c9aad5536eebef063669a99900e63

[chanli, 2016-07-12 20:55:50]

chanli@chromium.org changed reviewers:
+ lijeffrey@chromium.org, stgao@chromium.org

[chanli, 2016-07-12 20:55:51]

ptal

[stgao, 2016-07-12 21:09:12]

lgtm with nits.

https://codereview.chromium.org/2147713002/diff/1/appengine/findit/model/wf_s...
File appengine/findit/model/wf_swarming_task.py (right):

https://codereview.chromium.org/2147713002/diff/1/appengine/findit/model/wf_s...
appengine/findit/model/wf_swarming_task.py:81: task.parameters = {}
Could be simplified as below. Same for others.

  task.parameters = task.parameters or {}

BTW, do we need to check here right after creation?

[lijeffrey, 2016-07-12 21:15:39]

https://codereview.chromium.org/2147713002/diff/1/appengine/findit/model/wf_s...
File appengine/findit/model/wf_swarming_task.py (right):

https://codereview.chromium.org/2147713002/diff/1/appengine/findit/model/wf_s...
appengine/findit/model/wf_swarming_task.py:81: task.parameters = {}
On 2016/07/12 21:09:12, stgao wrote:
> Could be simplified as below. Same for others.
> 
>   task.parameters = task.parameters or {}
> 
> BTW, do we need to check here right after creation?

+1, since default={} has been removed, shouldn't these be None upon creation?

If it's safe to remove these then this code can be simplified furher to just

return WfSwarmingTask(key=...)

[chanli, 2016-07-12 23:45:45]

https://codereview.chromium.org/2147713002/diff/1/appengine/findit/model/wf_s...
File appengine/findit/model/wf_swarming_task.py (right):

https://codereview.chromium.org/2147713002/diff/1/appengine/findit/model/wf_s...
appengine/findit/model/wf_swarming_task.py:81: task.parameters = {}
On 2016/07/12 21:15:39, lijeffrey wrote:
> On 2016/07/12 21:09:12, stgao wrote:
> > Could be simplified as below. Same for others.
> > 
> >   task.parameters = task.parameters or {}
> > 
> > BTW, do we need to check here right after creation?
> 
> +1, since default={} has been removed, shouldn't these be None upon creation?
> 
> If it's safe to remove these then this code can be simplified furher to just
> 
> return WfSwarmingTask(key=...)

This change is to make sure we can directly add data to the dict without
checking if it's None first.

[lijeffrey, 2016-07-13 00:15:51]

lgtm

[stgao, 2016-07-13 06:51:31]

https://codereview.chromium.org/2147713002/diff/1/appengine/findit/model/wf_s...
File appengine/findit/model/wf_swarming_task.py (right):

https://codereview.chromium.org/2147713002/diff/1/appengine/findit/model/wf_s...
appengine/findit/model/wf_swarming_task.py:81: task.parameters = {}
On 2016/07/12 23:45:45, chanli wrote:
> On 2016/07/12 21:15:39, lijeffrey wrote:
> > On 2016/07/12 21:09:12, stgao wrote:
> > > Could be simplified as below. Same for others.
> > > 
> > >   task.parameters = task.parameters or {}
> > > 
> > > BTW, do we need to check here right after creation?
> > 
> > +1, since default={} has been removed, shouldn't these be None upon
creation?
> > 
> > If it's safe to remove these then this code can be simplified furher to just
> > 
> > return WfSwarmingTask(key=...)
> 
> This change is to make sure we can directly add data to the dict without
> checking if it's None first. 

Upon creation, the values are always None though.

[chanli, 2016-07-13 17:53:38]

https://codereview.chromium.org/2147713002/diff/1/appengine/findit/model/wf_s...
File appengine/findit/model/wf_swarming_task.py (right):

https://codereview.chromium.org/2147713002/diff/1/appengine/findit/model/wf_s...
appengine/findit/model/wf_swarming_task.py:81: task.parameters = {}
On 2016/07/13 06:51:31, stgao wrote:
> On 2016/07/12 23:45:45, chanli wrote:
> > On 2016/07/12 21:15:39, lijeffrey wrote:
> > > On 2016/07/12 21:09:12, stgao wrote:
> > > > Could be simplified as below. Same for others.
> > > > 
> > > >   task.parameters = task.parameters or {}
> > > > 
> > > > BTW, do we need to check here right after creation?
> > > 
> > > +1, since default={} has been removed, shouldn't these be None upon
> creation?
> > > 
> > > If it's safe to remove these then this code can be simplified furher to
just
> > > 
> > > return WfSwarmingTask(key=...)
> > 
> > This change is to make sure we can directly add data to the dict without
> > checking if it's None first. 
> 
> Upon creation, the values are always None though.

Yes, it just we don't need to always check if they are None when using them,
especially when adding data to them.

[chanli, 2016-07-19 18:07:13]

The CQ bit was checked by chanli@chromium.org

[chanli, 2016-07-19 18:07:13]

The patchset sent to the CQ was uploaded after l-g-t-m from stgao@chromium.org
Link to the patchset: https://codereview.chromium.org/2147713002/#ps40001
(title: ".")

[commit-bot: I haz the power, 2016-07-19 18:07:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-19 18:24:20]

Description was changed from

==========
[Findit] Make sure not use empty list or dict as default value.

BUG=
==========

to

==========
[Findit] Make sure not use empty list or dict as default value.

BUG=

Committed:
https://chromium.googlesource.com/infra/infra/+/d7b4ad09657c9aad5536eebef0636...
==========

[commit-bot: I haz the power, 2016-07-19 18:24:21]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/infra/infra/+/d7b4ad09657c9aad5536eebef0636...