Delete three class declarations that we don't provide implementations anymore.

Delete three class declarations that we don't provide implementations anymore.

This can be removed now that https://codereview.appspot.com/6650055/ was landed
in r5942.

R=epoger@google.com,reed@google.com

Committed: https://code.google.com/p/skia/source/detail?r=5967

[tfarina1, 2012-10-12 23:54:15]

Hi Elliot, could you review this to me?

[tfarina1, 2012-10-16 02:03:20]

Hi Mike, could you approve this?

[reed1, 2012-10-16 14:18:28]

removing dead declarations is fine. I don't agree with the removal of ////
dividers.

[tfarina1, 2012-10-16 17:01:30]

On 2012/10/16 14:18:28, reed1 wrote:
> removing dead declarations is fine. I don't agree with the removal of ////
> dividers.

Which removal of //// dividers are you talking about? What I see in the diff is
the opposite. I made it consistent and I added one between each class
declaration.

Could you take another look?

[reed1, 2012-10-16 17:21:39]

ah, you are correct about the direction of the change. my mistake.

Let me restate my earlier comment more generally. CLs that have a lot of
decorative/white-space changes take time to review, but offer very little value.
In the future we as a team will be more enthusiastic about code-reviews if there
are bugs getting fixes, or performance being improved.

lgtm

[epoger, 2012-10-29 19:31:26]

On 2012/10/16 17:21:39, reed1 wrote:
> Let me restate my earlier comment more generally. CLs that have a lot of
> decorative/white-space changes take time to review, but offer very little
value.
> In the future we as a team will be more enthusiastic about code-reviews if
there
> are bugs getting fixes, or performance being improved.

A question for you, Mike:

I agree that purely-cleanup changes typically provide little value (compared to
the effort involved in reviewing them, and the risk that they will have some bad
side-effect we don't discover until a couple days later).

But what if a developer is planning to make changes in a file, and wants to
clean up as he goes rather than adding more ugliness to an existing mess?

We don't want people to add a bunch of cleanup to a "real" CL, because it will
make it harder to review the "meat" of the CL.

I think in that case, it makes sense to either:
A: Create a purely-cleanup CL first, and include a message to the effect of "I'm
planning to make some changes in here, so I wanted to clean up a bit first",
or...
B: Add to the ugliness, and add TODOs describing the ugliness and recommending a
cleanup later.  (Then, after the "real" CL is in, create the cleanup CL.)

What do you think?

As an example, I submit my recent progression of CLs:

1. https://codereview.appspot.com/6801043 ('object-ify gmmain.cpp (no functional
change) to ease planned changes later')
Purely cleanup, because I knew I wanted to create CL#2.

2. https://codereview.appspot.com/6810047 ('Add option to gm: write out images
into a hierarchy, rather than a flat set of files')
Functional change which added a bit to the existing ugliness, but added some
TODO comments as I did so.  It was a lot cleaner than it WOULD have been if CL#1
hadn't gone in first.

3. https://codereview.appspot.com/6817050 ('Alphabetize gm options (in usage
message and option parser).')
Resolved the TODOs added in CL#2.

[tfarina1, 2012-10-29 19:48:25]

On Mon, Oct 29, 2012 at 5:31 PM,  <epoger@google.com> wrote:
> On 2012/10/16 17:21:39, reed1 wrote:
>>
>> Let me restate my earlier comment more generally. CLs that have a lot
>
> of
>>
>> decorative/white-space changes take time to review, but offer very
>
> little value.
>>
>> In the future we as a team will be more enthusiastic about
>
> code-reviews if there
>>
>> are bugs getting fixes, or performance being improved.
>
>
> A question for you, Mike:
>
> I agree that purely-cleanup changes typically provide little value

Yep, I have had planned a CL that removes duplicated header includes
from a bunch of files, but then I gave up.

> (compared to the effort involved in reviewing them, and the risk that
> they will have some bad side-effect we don't discover until a couple
> days later).
>


-- 
Thiago