Annotate overridden functions with SK_OVERRIDE in OverView class.

Annotate overridden functions with SK_OVERRIDE in OverView class.

R=bsalomon@google.com

Signed-off-by: Thiago Farina <tfarina@chromium.org>

Committed: http://code.google.com/p/skia/source/detail?r=3341

[bsalomon, 2012-03-06 16:44:57]

LGTM aside from my minor comments. It's an improvement but can I ask why?

https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp
File samplecode/OverView.cpp (left):

https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp#oldc...
samplecode/OverView.cpp:108: 
Is there still a final LF? I can't tell from the diff. I believe some compiler
complains if files don't end with one (even if its a cpp never included in
another file).

https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp
File samplecode/OverView.cpp (right):

https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp#newc...
samplecode/OverView.cpp:11: #include "SkTypes.h"
Is SkTypes required? We usually don't include it directly except in headers near
the top of the dependency graph.

[tfarina1, 2012-03-06 16:53:19]

On 2012/03/06 16:44:57, bsalomon wrote:
> LGTM aside from my minor comments. It's an improvement but can I ask why?
> 
The answer for the why question is
http://code.google.com/p/skia/issues/detail?id=319

I already have contributed with one here:
http://code.google.com/p/skia/source/detail?r=3318

But also to get more used to the skia code.

https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp
File samplecode/OverView.cpp (left):

https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp#oldc...
samplecode/OverView.cpp:108: 
On 2012/03/06 16:44:57, bsalomon wrote:
> Is there still a final LF? 
I think so.

> I can't tell from the diff. I believe some compiler
> complains if files don't end with one (even if its a cpp never included in
> another file).

I never had problems when removing it. My vim settings complains and this always
annoys me.

https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp
File samplecode/OverView.cpp (right):

https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp#newc...
samplecode/OverView.cpp:11: #include "SkTypes.h"
On 2012/03/06 16:44:57, bsalomon wrote:
> Is SkTypes required?
No, it compiles fine without it.

> We usually don't include it directly except in headers near
> the top of the dependency graph.

It's because of my tendency of iwyu
(http://code.google.com/p/include-what-you-use/)

[bsalomon, 2012-03-08 13:30:15]

Sorry I thought I had replied the other day. I must have quit the tab before
hitting submit.

On 2012/03/06 16:53:19, tfarina1 wrote:
> On 2012/03/06 16:44:57, bsalomon wrote:
> > LGTM aside from my minor comments. It's an improvement but can I ask why?
> > 
> The answer for the why question is
> http://code.google.com/p/skia/issues/detail?id=319
> 
> I already have contributed with one here:
> http://code.google.com/p/skia/source/detail?r=3318
> 
> But also to get more used to the skia code.
> 
> https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp
> File samplecode/OverView.cpp (left):
> 
>
https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp#oldc...
> samplecode/OverView.cpp:108: 
> On 2012/03/06 16:44:57, bsalomon wrote:
> > Is there still a final LF? 
> I think so.

Ok, I believe it is clang that complains about ending a file without an LF.

> 
> > I can't tell from the diff. I believe some compiler
> > complains if files don't end with one (even if its a cpp never included in
> > another file).
> 
> I never had problems when removing it. My vim settings complains and this
always
> annoys me.
> 
> https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp
> File samplecode/OverView.cpp (right):
> 
>
https://codereview.appspot.com/5752057/diff/1001/samplecode/OverView.cpp#newc...
> samplecode/OverView.cpp:11: #include "SkTypes.h"
> On 2012/03/06 16:44:57, bsalomon wrote:
> > Is SkTypes required?
> No, it compiles fine without it.
> 
> > We usually don't include it directly except in headers near
> > the top of the dependency graph.
> 
> It's because of my tendency of iwyu
> (http://code.google.com/p/include-what-you-use/)

I think consistency trumps establishing IWYU consistency in this one particular
not particularly important file. If we're going to have a strict IWYU policy in
skia then I'd prefer a bulk change (and some way of on-going verification).

[tfarina1, 2012-03-08 15:58:17]

Hi Brian, could you land this trivial patch for me?

[bsalomon, 2012-03-08 16:20:34]

Committed at r3341.