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

[Stephan Ewen]

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

[Ufuk Celebi-2]

On 08 Mar 2015, at 15:05, Stephan Ewen <[hidden email]> wrote:

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

+1 I'm always using Guava for the same reasons.

[Aljoscha Krettek-2]

+1 I also tend to use guava.

On Sun, Mar 8, 2015 at 3:21 PM, Ufuk Celebi <[hidden email]> wrote:

>
> On 08 Mar 2015, at 15:05, Stephan Ewen <[hidden email]> wrote:
>
>> 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)
>
> +1 I'm always using Guava for the same reasons.

... [show rest of quote]

[Robert Metzger]

I created a "starter" task JIRA for this.
https://issues.apache.org/jira/browse/FLINK-1787


On Sun, Mar 8, 2015 at 3:23 PM, Aljoscha Krettek <[hidden email]>
wrote:

> +1 I also tend to use guava.
>
> On Sun, Mar 8, 2015 at 3:21 PM, Ufuk Celebi <[hidden email]> wrote:
> >
> > On 08 Mar 2015, at 15:05, Stephan Ewen <[hidden email]> wrote:
> >
> >> 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)
> >
> > +1 I'm always using Guava for the same reasons.
>

... [show rest of quote]

[Robert Metzger]

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

[Robert Metzger]

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 <[hidden email]>
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
>

[Lokesh Rajaram]

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 <[hidden email]> 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 <[hidden email]>
> 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
>>

... [show rest of quote]

[Ufuk Celebi-2]

On 02 Jun 2015, at 21:18, Lokesh Rajaram <[hidden email]> 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

[Robert Metzger]

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 <[hidden email]>
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 <[hidden email]> 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 <[hidden email]>
> > 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
> >>
>

... [show rest of quote]

[Lokesh Rajaram]

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 <[hidden email]> 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 <[hidden email]>
> 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 <[hidden email]>
> 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 <[hidden email]>
> > > 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
> > >>
> >
>

... [show rest of quote]

[Robert Metzger]

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 <[hidden email]>
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 <[hidden email]>
> 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 <[hidden email]
> >
> > 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 <[hidden email]>
> > 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 <
> [hidden email]>
> > > > 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
> > > >>
> > >
> >
>

... [show rest of quote]

[Lokesh Rajaram]

Awesome. Will create a JIRA and assign it to me.

Thanks,
Lokesh

On Wed, Jun 3, 2015 at 8:03 AM, Robert Metzger <[hidden email]> 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 <[hidden email]>
> 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 <[hidden email]>
> > 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 <
> [hidden email]
> > >
> > > 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 <[hidden email]>
> > > 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 <
> > [hidden email]>
> > > > > 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
> > > > >>
> > > >
> > >
> >
>

... [show rest of quote]

[Ufuk Celebi-2]

Can you please also mark everything in the

org.apache.flink.shaded.*

namespace as illegal?

On 03 Jun 2015, at 17:08, Lokesh Rajaram <[hidden email]> wrote:

> Awesome. Will create a JIRA and assign it to me.
>
> Thanks,
> Lokesh
>
> On Wed, Jun 3, 2015 at 8:03 AM, Robert Metzger <[hidden email]> 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 <[hidden email]>
>> 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 <[hidden email]>
>>> 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 <
>> [hidden email]
>>>>
>>>> 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 <[hidden email]>
>>>> 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 <
>>> [hidden email]>
>>>>>> 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
>>>>>>>
>>>>>
>>>>
>>>
>>

... [show rest of quote]

[Lokesh Rajaram]

Sure, will add this to the list.

On Wed, Jun 3, 2015 at 8:20 AM, Ufuk Celebi <[hidden email]> 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 <[hidden email]> wrote:
>
> > Awesome. Will create a JIRA and assign it to me.
> >
> > Thanks,
> > Lokesh
> >
> > On Wed, Jun 3, 2015 at 8:03 AM, Robert Metzger <[hidden email]>
> 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 <
> [hidden email]>
> >> 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 <[hidden email]>
> >>> 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 <
> >> [hidden email]
> >>>>
> >>>> 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 <[hidden email]>
> >>>> 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 <
> >>> [hidden email]>
> >>>>>> 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
> >>>>>>>
> >>>>>
> >>>>
> >>>
> >>
>
>

... [show rest of quote]