Issue 6943059: Address comments for r6822: https://codereview.appspot.com/6946052/

Issue 6943059: Address comments for r6822: https://codereview.appspot.com/6946052/ (Closed)

Can't Edit
Can't Publish+Mail
Start Review

Created:
11 years, 11 months ago by EricB

Modified:
11 years, 11 months ago

Reviewers:
epoger, rmistry

CC:
skia-review_googlegroups.com

Base URL:
http://skia.googlecode.com/svn/trunk/

Visibility:
Public.

More Reviews

Description

Address comments for r6822: https://codereview.appspot.com/6946052/ Committed: https://code.google.com/p/skia/source/detail?r=6847

Patch Set 2 : #

Created: 11 years, 11 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+23 lines, -13 lines)			Patch
	M	tools/bench_pictures.cfg	View	1	1 chunk	+5 lines, -5 lines	0 comments	Download
	M	tools/tests/bench_pictures_cfg_test.py	View		1 chunk	+18 lines, -8 lines	0 comments	Download

Messages

Total messages: 4

Expand All Messages | Collapse All Messages

EricB

https://codereview.appspot.com/6943059/diff/1/tools/bench_pictures.cfg File tools/bench_pictures.cfg (right): https://codereview.appspot.com/6943059/diff/1/tools/bench_pictures.cfg#newcode63 tools/bench_pictures.cfg:63: TileGridConfig(DEFAULT_TILE_X, DEFAULT_TILE_Y), Just re-ordering these makes them much easier ...

11 years, 11 months ago (2012-12-14 21:11:13 UTC) #1

epoger

LGTM with one pushback https://codereview.appspot.com/6943059/diff/1/tools/bench_pictures.cfg File tools/bench_pictures.cfg (right): https://codereview.appspot.com/6943059/diff/1/tools/bench_pictures.cfg#newcode63 tools/bench_pictures.cfg:63: TileGridConfig(DEFAULT_TILE_X, DEFAULT_TILE_Y), On 2012/12/14 21:11:13, ...

11 years, 11 months ago (2012-12-17 16:51:22 UTC) #2

EricB

Made requested edit and committed as r6847. https://codereview.appspot.com/6943059/diff/1/tools/bench_pictures.cfg File tools/bench_pictures.cfg (right): https://codereview.appspot.com/6943059/diff/1/tools/bench_pictures.cfg#newcode63 tools/bench_pictures.cfg:63: TileGridConfig(DEFAULT_TILE_X, DEFAULT_TILE_Y), ...

11 years, 11 months ago (2012-12-17 17:22:15 UTC) #3

epoger

11 years, 11 months ago (2012-12-18 20:42:48 UTC) #4

Message was sent while issue was closed.

On 2012/12/17 17:22:15, EricB wrote:
> Made requested edit and committed as r6847.
> 
> https://codereview.appspot.com/6943059/diff/1/tools/bench_pictures.cfg
> File tools/bench_pictures.cfg (right):
> 
>
https://codereview.appspot.com/6943059/diff/1/tools/bench_pictures.cfg#newcode63
> tools/bench_pictures.cfg:63: TileGridConfig(DEFAULT_TILE_X, DEFAULT_TILE_Y),
> On 2012/12/17 16:51:23, epoger wrote:
> > On 2012/12/14 21:11:13, EricB wrote:
> > > Just re-ordering these makes them much easier to read.
> > 
> > Yes, but IMHO it would still be easier to read if all those repetitions of
> > DEFAULT_TILE_X and DEFAULT_TILE_Y lined up!
> 
> You win!  Done.

Yay, I win!  VICTORY

Expand All Messages | Collapse All Messages