[webkit-reviews] review granted: [Bug 63116] nrwt: allow for multiple http shards : [Attachment 98571] try to make _resize_shards clearer

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Dirk Pranke <dpranke at chromium.org>'s
request for review:
Bug 63116: nrwt: allow for multiple http shards
https://bugs.webkit.org/show_bug.cgi?id=63116

Attachment 98571: try to make _resize_shards clearer
https://bugs.webkit.org/attachment.cgi?id=98571&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98571&action=review

> Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:241
> +    # FIXME: make this class visible, used by workers as well.

Nit: make -> Make

> Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:244
> +    def __init__(self, name, test_inputs):
> +	   self.name = name
> +	   self.test_inputs = test_inputs

Nit: Maybe document that test_inputs is a list of TestInputs?

> Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:610
> +	   locked_shards = []
> +	   unlocked_shards = []

Nit: I would move these lines between the for loop to so it's closer to where
it's first used.

> Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:629
> +	   locked_shards.sort(key=lambda shard: shard.name)
> +	   unlocked_shards.sort(key=lambda shard: shard.name)

Should we implement __cmp__ for TestShard?

> Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:657
> +	       return int(math.ceil(numerator * 1.0 / divisor))

Nit: I think float(numerator) is more explicit than * 1.0.

> Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py:759
> +	       # FIXME: change 'test_list' to 'shard', make sharding public

Nit: change -> Change and public -> public.