incubator-crunch-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gabriel Reid <gabriel.r...@gmail.com>
Subject Re: Compiler warnings in Crunch
Date Wed, 20 Jun 2012 18:56:38 GMT
+1, I like the pragmatic approach, those warnings have no added value
and adding the serial UID everywhere is clutter.

I'll commit my current warning eradication efforts in a little bit --
getting rid of a lot of them, but there's still a big chunk to go.

- Gabriel

On Wed, Jun 20, 2012 at 7:36 PM, Josh Wills <jwills@cloudera.com> wrote:
> Yes, that does make life a lot nicer. :)
>
> Any objections to Tom's proposal? If not, I'll remove the UID lines
> from the code in a commit later today.
>
> J
>
> On Wed, Jun 20, 2012 at 10:31 AM, Tom White <tom@cloudera.com> wrote:
>> On Tue, Jun 19, 2012 at 4:19 PM, Josh Wills <jwills@cloudera.com> wrote:
>>> On Tue, Jun 19, 2012 at 1:56 PM, Tom White <tom@cloudera.com> wrote:
>>>> On Mon, Jun 18, 2012 at 6:42 PM, Robert Chu <robert@wibidata.com> wrote:
>>>>> +1 to fixing compiler warnings. I would prefer proper usage of the wildcard
>>>>> type to just suppressing the warnings.
>>>>
>>>> +1 Suppressing generics warnings should be the means of last resort,
>>>> and should be done at the smallest possible scope.
>>>>
>>>> Regarding the serialization warnings, I think it's better not to add
>>>> serial UIDs everywhere since they add clutter. You can turn off the
>>>> warnings in Eclipse instead - would that acceptable?
>>>
>>> I'm okay with that-- is it a setting we can add to the config settings
>>> Gabriel just checked in?
>>
>> Unfortunately it doesn't look like it - those are formatting rules,
>> and they don't affect the compiler. You can turn the warning off in
>> Preferences: Java Compiler -> Errors/Warnings -> Potential programming
>> problems -> Serializable class without serialVersionUID. Change the
>> drop down to "Ignore".
>>
>> Cheers,
>> Tom
>>
>>>
>>>>
>>>> Cheers,
>>>> Tom
>>>>
>>>>>
>>>>> Robert
>>>>>
>>>>> On Mon, Jun 18, 2012 at 7:57 AM, Josh Wills <jwills@cloudera.com>
wrote:
>>>>>
>>>>>> On Mon, Jun 18, 2012 at 6:24 AM, Gabriel Reid <gabriel.reid@gmail.com>
>>>>>> wrote:
>>>>>> > As prep for some other development I was going to do in Crunch,
I was
>>>>>> > considering cleaning up some (or all) of the compiler warnings
that
>>>>>> > are currently occurring (they make the right-side search ribbon
in
>>>>>> > Eclipse almost unusable right now).
>>>>>> >
>>>>>> > A significant portion of the compiler warnings are raw type
generics
>>>>>> > warnings, i.e. "xxx is a raw type. References to xxx should
be
>>>>>> > parameterized", where we're doing general operations with PCollections
>>>>>> > and similar objects without knowing anything about their generic
>>>>>> > types.
>>>>>>
>>>>>> There are also the warnings about not adding serialization UIDs to
the
>>>>>> built-in DoFns, which irritate me and are useless in the context
of
>>>>>> Crunch. Happy to volunteer to go around and add UID = 1; code all
over
>>>>>> the place if there are no objections.
>>>>>>
>>>>>> >
>>>>>> > We could add wildcards (i.e. PCollection<?>) to each of
these generic
>>>>>> > objects in the methods where the warnings are occurring -- this
would
>>>>>> > be my preferred thing to do. On the other hand, I think that
adding
>>>>>> > wildcards make the code more difficult to read, while having
any kind
>>>>>> > of real added value.
>>>>>> >
>>>>>> > The other option we could take (less preferable to me) is to
use
>>>>>> > @SuppressWarnings("rawtypes") on some or all of the affected
methods
>>>>>> > -- it'll leave the code in a more readable state, but I'm not
that
>>>>>> > wild about just suppressing warnings.
>>>>>>
>>>>>> I'm a 0 on the approach here-- I always have a hard time getting
the
>>>>>> <?> stuff to compile when I'm casting the result, which is
what often
>>>>>> happens in Writables.java and Avros.java, but I agree that a working
>>>>>> version of the wildcards is preferable to suppress warnings. We might
>>>>>> say that we prefer <?> but add in SuppressWarnings when there
is no
>>>>>> other option for what we're trying to do.
>>>>>>
>>>>>> >
>>>>>> > Anyone else care to weigh in on this?
>>>>>> >
>>>>>> > - Gabriel
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Director of Data Science
>>>>>> Cloudera
>>>>>> Twitter: @josh_wills
>>>>>>
>>>
>>>
>>>
>>> --
>>> Director of Data Science
>>> Cloudera
>>> Twitter: @josh_wills
>
>
>
> --
> Director of Data Science
> Cloudera
> Twitter: @josh_wills

Mime
View raw message