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