You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@flink.apache.org by Stephan Ewen <se...@apache.org> on 2015/03/08 15:05:18 UTC

"Validate" (commons) versus "checkArgument" (guava)

Different parts of the code currently use different utilities to validate
the arguments.

  - Some parts use Guava (checkNotNull, checkArgument)
  - Other parts use Validate from Apache commons-lang(3).

How about we use one consistently, at least for all new code additions?

In choosing one, I have a slight bias towards Guava, which has more/nicer
methods and seems more popular in other projects (I have no source to back
this up, it is a gut feeling from what I have seen in other projects that I
looked into)

Greetings,
Stephan

Re: "Validate" (commons) versus "checkArgument" (guava)

Posted by Ufuk Celebi <uc...@apache.org>.

On 02 Jun 2015, at 21:18, Lokesh Rajaram <ra...@gmail.com> wrote:

> Hello Robert,
> 
> I worked on that issue, if it's ok I can take this task.
> 
> Btw, how is anything enforced in Flink? Do I have to update how to contribute guide or any thing else need to be done?

The how to contribute guide is a good start. You can also have a look at the QA bot in ./tools/qa-check.sh. The bot is not activated, but it would be the best to enforce it automatically in the long term. You can also add a check that ensure that no "shaded" depencies are imported. This happened once and failed the builds.

If you want this written down, I can open an issue for it and assign it to you.

– Ufuk

Re: "Validate" (commons) versus "checkArgument" (guava)

Posted by Lokesh Rajaram <ra...@gmail.com>.

Sure, will add this to the list.

On Wed, Jun 3, 2015 at 8:20 AM, Ufuk Celebi <uc...@apache.org> wrote:

> Can you please also mark everything in the
>
> org.apache.flink.shaded.*
>
> namespace as illegal?
>
> On 03 Jun 2015, at 17:08, Lokesh Rajaram <ra...@gmail.com> wrote:
>
> > Awesome. Will create a JIRA and assign it to me.
> >
> > Thanks,
> > Lokesh
> >
> > On Wed, Jun 3, 2015 at 8:03 AM, Robert Metzger <rm...@apache.org>
> wrote:
> >
> >> Hi,
> >>
> >> the illegalImport check is exactly what I had in mind. Would be great if
> >> you could add such a check.
> >> You can also create JIRA issues yourself.
> >>
> >> On Wed, Jun 3, 2015 at 6:39 AM, Lokesh Rajaram <
> rajaram.lokesh@gmail.com>
> >> wrote:
> >>
> >>> Hello Ufuk, Robert,
> >>>
> >>> @Ufuk if you can create a ticket and assign it to me that would be very
> >>> helpful
> >>>
> >>> @Robert I can definitely update those two documents. Just now I was
> >>> checking and looks like we can control this using checkstyle
> >>> http://checkstyle.sourceforge.net/config_imports.html#IllegalImport
> >>>
> >>> Let me know if this is similar to what you guys were looking for.
> >>>
> >>> Thanks,
> >>> Lokesh
> >>>
> >>> On Tue, Jun 2, 2015 at 12:21 PM, Robert Metzger <rm...@apache.org>
> >>> wrote:
> >>>
> >>>> Adding an entry here: http://flink.apache.org/coding-guidelines.html
> >> is
> >>>> certainly good, yes.
> >>>> You can contribute to the website here:
> >>>> https://github.com/apache/flink-web
> >>>>
> >>>> We enforce coding guidelines using the maven checkstyle plugin. Maybe
> >>> there
> >>>> is a way of forbidding certain imports
> >>>>
> >>>> On Tue, Jun 2, 2015 at 9:18 PM, Lokesh Rajaram <
> >> rajaram.lokesh@gmail.com
> >>>>
> >>>> wrote:
> >>>>
> >>>>> Hello Robert,
> >>>>>
> >>>>> I worked on that issue, if it's ok I can take this task.
> >>>>>
> >>>>> Btw, how is anything enforced in Flink? Do I have to update how to
> >>>>> contribute guide or any thing else need to be done?
> >>>>>
> >>>>> Sent from my iPhone
> >>>>>
> >>>>>> On Jun 2, 2015, at 12:11 PM, Robert Metzger <rm...@apache.org>
> >>>> wrote:
> >>>>>>
> >>>>>> We have now replaced all commons validate calls with guava
> >>>> preconditions
> >>>>>> but its not written down anywhere or enforced by anything.
> >>>>>>
> >>>>>> Who would like to take care of that?
> >>>>>>
> >>>>>> On Thu, Mar 26, 2015 at 11:03 AM, Robert Metzger <
> >>> rmetzger@apache.org>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> I didn't know that there was already an issue for this. I closed
> >>>>>>> FLINK-1787.
> >>>>>>> The correct issue is this one:
> >>>>>>> https://issues.apache.org/jira/browse/FLINK-1711
> >>>>>>>
> >>>>>
> >>>>
> >>>
> >>
>
>

Re: "Validate" (commons) versus "checkArgument" (guava)

Posted by Ufuk Celebi <uc...@apache.org>.

Can you please also mark everything in the

org.apache.flink.shaded.*

namespace as illegal?

On 03 Jun 2015, at 17:08, Lokesh Rajaram <ra...@gmail.com> wrote:

> Awesome. Will create a JIRA and assign it to me.
> 
> Thanks,
> Lokesh
> 
> On Wed, Jun 3, 2015 at 8:03 AM, Robert Metzger <rm...@apache.org> wrote:
> 
>> Hi,
>> 
>> the illegalImport check is exactly what I had in mind. Would be great if
>> you could add such a check.
>> You can also create JIRA issues yourself.
>> 
>> On Wed, Jun 3, 2015 at 6:39 AM, Lokesh Rajaram <ra...@gmail.com>
>> wrote:
>> 
>>> Hello Ufuk, Robert,
>>> 
>>> @Ufuk if you can create a ticket and assign it to me that would be very
>>> helpful
>>> 
>>> @Robert I can definitely update those two documents. Just now I was
>>> checking and looks like we can control this using checkstyle
>>> http://checkstyle.sourceforge.net/config_imports.html#IllegalImport
>>> 
>>> Let me know if this is similar to what you guys were looking for.
>>> 
>>> Thanks,
>>> Lokesh
>>> 
>>> On Tue, Jun 2, 2015 at 12:21 PM, Robert Metzger <rm...@apache.org>
>>> wrote:
>>> 
>>>> Adding an entry here: http://flink.apache.org/coding-guidelines.html
>> is
>>>> certainly good, yes.
>>>> You can contribute to the website here:
>>>> https://github.com/apache/flink-web
>>>> 
>>>> We enforce coding guidelines using the maven checkstyle plugin. Maybe
>>> there
>>>> is a way of forbidding certain imports
>>>> 
>>>> On Tue, Jun 2, 2015 at 9:18 PM, Lokesh Rajaram <
>> rajaram.lokesh@gmail.com
>>>> 
>>>> wrote:
>>>> 
>>>>> Hello Robert,
>>>>> 
>>>>> I worked on that issue, if it's ok I can take this task.
>>>>> 
>>>>> Btw, how is anything enforced in Flink? Do I have to update how to
>>>>> contribute guide or any thing else need to be done?
>>>>> 
>>>>> Sent from my iPhone
>>>>> 
>>>>>> On Jun 2, 2015, at 12:11 PM, Robert Metzger <rm...@apache.org>
>>>> wrote:
>>>>>> 
>>>>>> We have now replaced all commons validate calls with guava
>>>> preconditions
>>>>>> but its not written down anywhere or enforced by anything.
>>>>>> 
>>>>>> Who would like to take care of that?
>>>>>> 
>>>>>> On Thu, Mar 26, 2015 at 11:03 AM, Robert Metzger <
>>> rmetzger@apache.org>
>>>>>> wrote:
>>>>>> 
>>>>>>> I didn't know that there was already an issue for this. I closed
>>>>>>> FLINK-1787.
>>>>>>> The correct issue is this one:
>>>>>>> https://issues.apache.org/jira/browse/FLINK-1711
>>>>>>> 
>>>>> 
>>>> 
>>> 
>>