incubator-crunch-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Josh Wills <jwi...@cloudera.com>
Subject Re: Compiler warnings in Crunch
Date Wed, 20 Jun 2012 19:03:57 GMT
On Wed, Jun 20, 2012 at 11:56 AM, Gabriel Reid <gabriel.reid@gmail.com> wrote:
> +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.

Sounds good. I'll do the serial UID cleanup after that's in.

>
> - 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



-- 
Director of Data Science
Cloudera
Twitter: @josh_wills

Mime
View raw message