On Wed, Jun 20, 2012 at 11:56 AM, Gabriel Reid <[email protected]> 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 <[email protected]> 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 <[email protected]> wrote: >>> On Tue, Jun 19, 2012 at 4:19 PM, Josh Wills <[email protected]> wrote: >>>> On Tue, Jun 19, 2012 at 1:56 PM, Tom White <[email protected]> wrote: >>>>> On Mon, Jun 18, 2012 at 6:42 PM, Robert Chu <[email protected]> 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 <[email protected]> wrote: >>>>>> >>>>>>> On Mon, Jun 18, 2012 at 6:24 AM, Gabriel Reid <[email protected]> >>>>>>> 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
