Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
12502026:
Reformat to match chromium style. (Closed)
|
Created:
7 years, 9 months ago by alokp Modified:
7 years, 9 months ago CC:
chromium-reviews, cc-bugs_chromium.org, aelias_OOO_until_Jul13 Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionReformat layer_tree_host_unittest to match chromium style
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=190759
Patch Set 1 #Patch Set 2 : fixed lint errors #
Total comments: 15
Patch Set 3 : addressed comments and some more #Messages
Total messages: 13 (0 generated)
Verified that the file still builds and tests pass.
Can you run this through cpplint.py and fix all the errors there?
On 2013/03/25 18:33:37, enne wrote: > Can you run this through cpplint.py and fix all the errors there? Running cpplint locally did not work. But now that I can see the lint errors here, I will fix them.
On 2013/03/25 22:56:58, Alok Priyadarshi wrote: > On 2013/03/25 18:33:37, enne wrote: > > Can you run this through cpplint.py and fix all the errors there? > > Running cpplint locally did not work. But now that I can see the lint errors > here, I will fix them. What do you mean "did not work"? What did you try and what happened?
> What do you mean "did not work"? What did you try and what happened?
alokp@alokp:~/src/chrome/src$ cpplint_chromium.py
cc/trees/layer_tree_host_unittest.cc
/usr/local/google/home/alokp/src/depot_tools/cpplint_chromium.py: line 33:
syntax error near unexpected token `('
/usr/local/google/home/alokp/src/depot_tools/cpplint_chromium.py: line 33:
`_RE_PATTERN_POINTER_DECLARATION_WHITESPACE = re.compile('
The python version is 2.7.3. I probably need to file a bug for
cpplint_chromium.py?
On 2013/03/25 23:38:55, Alok Priyadarshi wrote:
> > What do you mean "did not work"? What did you try and what happened?
>
> alokp@alokp:~/src/chrome/src$ cpplint_chromium.py
> cc/trees/layer_tree_host_unittest.cc
> /usr/local/google/home/alokp/src/depot_tools/cpplint_chromium.py: line 33:
> syntax error near unexpected token `('
> /usr/local/google/home/alokp/src/depot_tools/cpplint_chromium.py: line 33:
> `_RE_PATTERN_POINTER_DECLARATION_WHITESPACE = re.compile('
>
> The python version is 2.7.3. I probably need to file a bug for
> cpplint_chromium.py?
Just run cpplint.py
BTW all lint errors are fixed in the new patch.
> Just run cpplint.py Ah. I did not know there was a cpplint.py in depot_tools. There is another one in /usr/bin. So I assumed cpplint_chromium.py is chromium-specific. Thanks.
Thanks for the patch. I know these long test files are a slog. https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... File cc/trees/layer_tree_host_unittest.cc (right): https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:153: if (!num_draws_) If the body of an if statement has > 1 line, then it needs {}. Also, the else then needs {} too. https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:491: impl->active_tree()->page_scale_factor(), 0.5, 2); 0.5f, 2.f (these are floating point literals, here and elsewhere in this file for SetPageScaleFactorAndLimits) https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:515: EXPECT_EQ(1.25, impl->active_tree()->page_scale_factor()); 1.25f https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:568: test_layer_->SetOpacity(0); This is a float (i.e. 0.f), here and in other SetOpacity calls in this file. https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:596: SetAnchorPoint(gfx::PointF(0, 0)); PointF(), here and elsewhere https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:703: EXPECT_NEAR(impl->device_scale_factor(), 1.5, 0.00001); 1.5f, 0.00001f https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:755: child_screen_space_transform.Translate(3, 3); 3.0, 3.0 https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:1808: EXPECT_EQ(FrameRateController::DEFAULT_MAX_FRAMES_PENDING, {} on the if and else
Also fixed a few other locations for formatting and float literals. https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... File cc/trees/layer_tree_host_unittest.cc (right): https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:153: if (!num_draws_) On 2013/03/26 01:26:15, enne wrote: > If the body of an if statement has > 1 line, then it needs {}. Also, the else > then needs {} too. Done. https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:491: impl->active_tree()->page_scale_factor(), 0.5, 2); On 2013/03/26 01:26:15, enne wrote: > 0.5f, 2.f (these are floating point literals, here and elsewhere in this file > for SetPageScaleFactorAndLimits) Done. https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:515: EXPECT_EQ(1.25, impl->active_tree()->page_scale_factor()); On 2013/03/26 01:26:15, enne wrote: > 1.25f Done. https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:568: test_layer_->SetOpacity(0); On 2013/03/26 01:26:15, enne wrote: > This is a float (i.e. 0.f), here and in other SetOpacity calls in this file. Done. https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:596: SetAnchorPoint(gfx::PointF(0, 0)); On 2013/03/26 01:26:15, enne wrote: > PointF(), here and elsewhere Done. https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:703: EXPECT_NEAR(impl->device_scale_factor(), 1.5, 0.00001); On 2013/03/26 01:26:15, enne wrote: > 1.5f, 0.00001f Done. https://codereview.chromium.org/12502026/diff/7001/cc/trees/layer_tree_host_u... cc/trees/layer_tree_host_unittest.cc:1808: EXPECT_EQ(FrameRateController::DEFAULT_MAX_FRAMES_PENDING, On 2013/03/26 01:26:15, enne wrote: > {} on the if and else Done.
lgtm, cq+
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/alokp@chromium.org/12502026/13001
Message was sent while issue was closed.
Change committed as 190759 |
