Running lint-java during PR builds?

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Running lint-java during PR builds?

Marcelo Vanzin
Hey all,

Is there a reason why lint-java is not run during PR builds? I see it
seems to be maven-only, is it really expensive to run after an sbt
build?

I see a lot of PRs coming in to fix Java style issues, and those all
seem a little unnecessary. Either we're enforcing style checks or
we're not, and right now it seems we aren't.

--
Marcelo

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Shixiong(Ryan) Zhu
I remember it's because you need to run `mvn install` before running lint-java if the maven cache is empty, and `mvn install` is pretty heavy.

On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]> wrote:
Hey all,

Is there a reason why lint-java is not run during PR builds? I see it
seems to be maven-only, is it really expensive to run after an sbt
build?

I see a lot of PRs coming in to fix Java style issues, and those all
seem a little unnecessary. Either we're enforcing style checks or
we're not, and right now it seems we aren't.

--
Marcelo

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Dongjoon Hyun
Hi, Marcelo and Ryan.

That was the main purpose of my proposal about Travis.CI.
IMO, that is the only way to achieve that without any harmful side-effect on Jenkins infra.

Spark is already ready for that. Like AppVoyer, if one of you files an INFRA jira issue to enable that, they will turn on that. Then, we can try it and see the result. Also, you can turn off easily again if you don't want.

Without this, we will consume more community efforts. For example, we merged lint-java error fix PR seven hours ago, but the master branch still has one lint-java error.

https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319

Actually, I've been monitoring the history here. (It's synced every 30 minutes.)

https://travis-ci.org/dongjoon-hyun/spark/builds

Could we give a change to this?

Bests,
Dongjoon.

On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu" <[hidden email]> wrote:

> I remember it's because you need to run `mvn install` before running
> lint-java if the maven cache is empty, and `mvn install` is pretty heavy.
>
> On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]> wrote:
>
> > Hey all,
> >
> > Is there a reason why lint-java is not run during PR builds? I see it
> > seems to be maven-only, is it really expensive to run after an sbt
> > build?
> >
> > I see a lot of PRs coming in to fix Java style issues, and those all
> > seem a little unnecessary. Either we're enforcing style checks or
> > we're not, and right now it seems we aren't.
> >
> > --
> > Marcelo
> >
> > ---------------------------------------------------------------------
> > To unsubscribe e-mail: [hidden email]
> >
> >
>

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Hyukjin Kwon
I would like to revive this proposal. Travis CI. Shall we give this try? I think it's worth trying it.

2016-11-17 3:50 GMT+08:00 Dongjoon Hyun <[hidden email]>:
Hi, Marcelo and Ryan.

That was the main purpose of my proposal about Travis.CI.
IMO, that is the only way to achieve that without any harmful side-effect on Jenkins infra.

Spark is already ready for that. Like AppVoyer, if one of you files an INFRA jira issue to enable that, they will turn on that. Then, we can try it and see the result. Also, you can turn off easily again if you don't want.

Without this, we will consume more community efforts. For example, we merged lint-java error fix PR seven hours ago, but the master branch still has one lint-java error.

https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319

Actually, I've been monitoring the history here. (It's synced every 30 minutes.)

https://travis-ci.org/dongjoon-hyun/spark/builds

Could we give a change to this?

Bests,
Dongjoon.

On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu" <[hidden email]> wrote:
> I remember it's because you need to run `mvn install` before running
> lint-java if the maven cache is empty, and `mvn install` is pretty heavy.
>
> On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]> wrote:
>
> > Hey all,
> >
> > Is there a reason why lint-java is not run during PR builds? I see it
> > seems to be maven-only, is it really expensive to run after an sbt
> > build?
> >
> > I see a lot of PRs coming in to fix Java style issues, and those all
> > seem a little unnecessary. Either we're enforcing style checks or
> > we're not, and right now it seems we aren't.
> >
> > --
> > Marcelo
> >
> > ---------------------------------------------------------------------
> > To unsubscribe e-mail: [hidden email]
> >
> >
>

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]


Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Hyukjin Kwon
I am going to open an INFRA JIRA if there's no explicit objection in few days.

2018-05-21 13:09 GMT+08:00 Hyukjin Kwon <[hidden email]>:
I would like to revive this proposal. Travis CI. Shall we give this try? I think it's worth trying it.

2016-11-17 3:50 GMT+08:00 Dongjoon Hyun <[hidden email]>:
Hi, Marcelo and Ryan.

That was the main purpose of my proposal about Travis.CI.
IMO, that is the only way to achieve that without any harmful side-effect on Jenkins infra.

Spark is already ready for that. Like AppVoyer, if one of you files an INFRA jira issue to enable that, they will turn on that. Then, we can try it and see the result. Also, you can turn off easily again if you don't want.

Without this, we will consume more community efforts. For example, we merged lint-java error fix PR seven hours ago, but the master branch still has one lint-java error.

https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319

Actually, I've been monitoring the history here. (It's synced every 30 minutes.)

https://travis-ci.org/dongjoon-hyun/spark/builds

Could we give a change to this?

Bests,
Dongjoon.

On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu" <[hidden email]> wrote:
> I remember it's because you need to run `mvn install` before running
> lint-java if the maven cache is empty, and `mvn install` is pretty heavy.
>
> On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]> wrote:
>
> > Hey all,
> >
> > Is there a reason why lint-java is not run during PR builds? I see it
> > seems to be maven-only, is it really expensive to run after an sbt
> > build?
> >
> > I see a lot of PRs coming in to fix Java style issues, and those all
> > seem a little unnecessary. Either we're enforcing style checks or
> > we're not, and right now it seems we aren't.
> >
> > --
> > Marcelo
> >
> > ---------------------------------------------------------------------
> > To unsubscribe e-mail: [hidden email]
> >
> >
>

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]



Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Marcelo Vanzin
I'm fine with it. I tried to use the existing checkstyle sbt plugin
(trying to fix SPARK-22269), but it depends on an ancient version of
checkstyle, and I don't know sbt enough to figure out how to hack
classpaths and class loaders when applying rules, so gave up.

On Mon, May 21, 2018 at 1:47 AM, Hyukjin Kwon <[hidden email]> wrote:

> I am going to open an INFRA JIRA if there's no explicit objection in few
> days.
>
> 2018-05-21 13:09 GMT+08:00 Hyukjin Kwon <[hidden email]>:
>>
>> I would like to revive this proposal. Travis CI. Shall we give this try? I
>> think it's worth trying it.
>>
>> 2016-11-17 3:50 GMT+08:00 Dongjoon Hyun <[hidden email]>:
>>>
>>> Hi, Marcelo and Ryan.
>>>
>>> That was the main purpose of my proposal about Travis.CI.
>>> IMO, that is the only way to achieve that without any harmful side-effect
>>> on Jenkins infra.
>>>
>>> Spark is already ready for that. Like AppVoyer, if one of you files an
>>> INFRA jira issue to enable that, they will turn on that. Then, we can try it
>>> and see the result. Also, you can turn off easily again if you don't want.
>>>
>>> Without this, we will consume more community efforts. For example, we
>>> merged lint-java error fix PR seven hours ago, but the master branch still
>>> has one lint-java error.
>>>
>>> https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319
>>>
>>> Actually, I've been monitoring the history here. (It's synced every 30
>>> minutes.)
>>>
>>> https://travis-ci.org/dongjoon-hyun/spark/builds
>>>
>>> Could we give a change to this?
>>>
>>> Bests,
>>> Dongjoon.
>>>
>>> On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu"
>>> <[hidden email]> wrote:
>>> > I remember it's because you need to run `mvn install` before running
>>> > lint-java if the maven cache is empty, and `mvn install` is pretty
>>> > heavy.
>>> >
>>> > On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]>
>>> > wrote:
>>> >
>>> > > Hey all,
>>> > >
>>> > > Is there a reason why lint-java is not run during PR builds? I see it
>>> > > seems to be maven-only, is it really expensive to run after an sbt
>>> > > build?
>>> > >
>>> > > I see a lot of PRs coming in to fix Java style issues, and those all
>>> > > seem a little unnecessary. Either we're enforcing style checks or
>>> > > we're not, and right now it seems we aren't.
>>> > >
>>> > > --
>>> > > Marcelo
>>> > >
>>> > > ---------------------------------------------------------------------
>>> > > To unsubscribe e-mail: [hidden email]
>>> > >
>>> > >
>>> >
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe e-mail: [hidden email]
>>>
>>
>



--
Marcelo

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Felix Cheung
One concern is with the volume of test runs on Travis.

In ASF projects Travis could get significantly
backed up since - if I recall - all of ASF shares one queue.

At the number of PRs Spark has this could be a big issue.



From: Marcelo Vanzin <[hidden email]>
Sent: Monday, May 21, 2018 9:08:28 AM
To: Hyukjin Kwon
Cc: Dongjoon Hyun; dev
Subject: Re: Running lint-java during PR builds?
 
I'm fine with it. I tried to use the existing checkstyle sbt plugin
(trying to fix SPARK-22269), but it depends on an ancient version of
checkstyle, and I don't know sbt enough to figure out how to hack
classpaths and class loaders when applying rules, so gave up.

On Mon, May 21, 2018 at 1:47 AM, Hyukjin Kwon <[hidden email]> wrote:
> I am going to open an INFRA JIRA if there's no explicit objection in few
> days.
>
> 2018-05-21 13:09 GMT+08:00 Hyukjin Kwon <[hidden email]>:
>>
>> I would like to revive this proposal. Travis CI. Shall we give this try? I
>> think it's worth trying it.
>>
>> 2016-11-17 3:50 GMT+08:00 Dongjoon Hyun <[hidden email]>:
>>>
>>> Hi, Marcelo and Ryan.
>>>
>>> That was the main purpose of my proposal about Travis.CI.
>>> IMO, that is the only way to achieve that without any harmful side-effect
>>> on Jenkins infra.
>>>
>>> Spark is already ready for that. Like AppVoyer, if one of you files an
>>> INFRA jira issue to enable that, they will turn on that. Then, we can try it
>>> and see the result. Also, you can turn off easily again if you don't want.
>>>
>>> Without this, we will consume more community efforts. For example, we
>>> merged lint-java error fix PR seven hours ago, but the master branch still
>>> has one lint-java error.
>>>
>>> https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319
>>>
>>> Actually, I've been monitoring the history here. (It's synced every 30
>>> minutes.)
>>>
>>> https://travis-ci.org/dongjoon-hyun/spark/builds
>>>
>>> Could we give a change to this?
>>>
>>> Bests,
>>> Dongjoon.
>>>
>>> On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu"
>>> <[hidden email]> wrote:
>>> > I remember it's because you need to run `mvn install` before running
>>> > lint-java if the maven cache is empty, and `mvn install` is pretty
>>> > heavy.
>>> >
>>> > On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]>
>>> > wrote:
>>> >
>>> > > Hey all,
>>> > >
>>> > > Is there a reason why lint-java is not run during PR builds? I see it
>>> > > seems to be maven-only, is it really expensive to run after an sbt
>>> > > build?
>>> > >
>>> > > I see a lot of PRs coming in to fix Java style issues, and those all
>>> > > seem a little unnecessary. Either we're enforcing style checks or
>>> > > we're not, and right now it seems we aren't.
>>> > >
>>> > > --
>>> > > Marcelo
>>> > >
>>> > > ---------------------------------------------------------------------
>>> > > To unsubscribe e-mail: [hidden email]
>>> > >
>>> > >
>>> >
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe e-mail: [hidden email]
>>>
>>
>



--
Marcelo

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Marcelo Vanzin
Is there a way to trigger it conditionally? e.g. only if the diff
touches java files.

On Mon, May 21, 2018 at 9:17 AM, Felix Cheung <[hidden email]> wrote:

> One concern is with the volume of test runs on Travis.
>
> In ASF projects Travis could get significantly
> backed up since - if I recall - all of ASF shares one queue.
>
> At the number of PRs Spark has this could be a big issue.
>
>
> ________________________________
> From: Marcelo Vanzin <[hidden email]>
> Sent: Monday, May 21, 2018 9:08:28 AM
> To: Hyukjin Kwon
> Cc: Dongjoon Hyun; dev
> Subject: Re: Running lint-java during PR builds?
>
> I'm fine with it. I tried to use the existing checkstyle sbt plugin
> (trying to fix SPARK-22269), but it depends on an ancient version of
> checkstyle, and I don't know sbt enough to figure out how to hack
> classpaths and class loaders when applying rules, so gave up.
>
> On Mon, May 21, 2018 at 1:47 AM, Hyukjin Kwon <[hidden email]> wrote:
>> I am going to open an INFRA JIRA if there's no explicit objection in few
>> days.
>>
>> 2018-05-21 13:09 GMT+08:00 Hyukjin Kwon <[hidden email]>:
>>>
>>> I would like to revive this proposal. Travis CI. Shall we give this try?
>>> I
>>> think it's worth trying it.
>>>
>>> 2016-11-17 3:50 GMT+08:00 Dongjoon Hyun <[hidden email]>:
>>>>
>>>> Hi, Marcelo and Ryan.
>>>>
>>>> That was the main purpose of my proposal about Travis.CI.
>>>> IMO, that is the only way to achieve that without any harmful
>>>> side-effect
>>>> on Jenkins infra.
>>>>
>>>> Spark is already ready for that. Like AppVoyer, if one of you files an
>>>> INFRA jira issue to enable that, they will turn on that. Then, we can
>>>> try it
>>>> and see the result. Also, you can turn off easily again if you don't
>>>> want.
>>>>
>>>> Without this, we will consume more community efforts. For example, we
>>>> merged lint-java error fix PR seven hours ago, but the master branch
>>>> still
>>>> has one lint-java error.
>>>>
>>>> https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319
>>>>
>>>> Actually, I've been monitoring the history here. (It's synced every 30
>>>> minutes.)
>>>>
>>>> https://travis-ci.org/dongjoon-hyun/spark/builds
>>>>
>>>> Could we give a change to this?
>>>>
>>>> Bests,
>>>> Dongjoon.
>>>>
>>>> On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu"
>>>> <[hidden email]> wrote:
>>>> > I remember it's because you need to run `mvn install` before running
>>>> > lint-java if the maven cache is empty, and `mvn install` is pretty
>>>> > heavy.
>>>> >
>>>> > On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]>
>>>> > wrote:
>>>> >
>>>> > > Hey all,
>>>> > >
>>>> > > Is there a reason why lint-java is not run during PR builds? I see
>>>> > > it
>>>> > > seems to be maven-only, is it really expensive to run after an sbt
>>>> > > build?
>>>> > >
>>>> > > I see a lot of PRs coming in to fix Java style issues, and those all
>>>> > > seem a little unnecessary. Either we're enforcing style checks or
>>>> > > we're not, and right now it seems we aren't.
>>>> > >
>>>> > > --
>>>> > > Marcelo
>>>> > >
>>>> > >
>>>> > > ---------------------------------------------------------------------
>>>> > > To unsubscribe e-mail: [hidden email]
>>>> > >
>>>> > >
>>>> >
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe e-mail: [hidden email]
>>>>
>>>
>>
>
>
>
> --
> Marcelo
>
> ---------------------------------------------------------------------
> To unsubscribe e-mail: [hidden email]
>



--
Marcelo

---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Dongjoon Hyun-2
Thank you for reconsidering this, Hyukjin. :)

Bests,
Dongjoon.


On Mon, May 21, 2018 at 9:20 AM, Marcelo Vanzin <[hidden email]> wrote:
Is there a way to trigger it conditionally? e.g. only if the diff
touches java files.

On Mon, May 21, 2018 at 9:17 AM, Felix Cheung <[hidden email]> wrote:
> One concern is with the volume of test runs on Travis.
>
> In ASF projects Travis could get significantly
> backed up since - if I recall - all of ASF shares one queue.
>
> At the number of PRs Spark has this could be a big issue.
>
>
> ________________________________
> From: Marcelo Vanzin <[hidden email]>
> Sent: Monday, May 21, 2018 9:08:28 AM
> To: Hyukjin Kwon
> Cc: Dongjoon Hyun; dev
> Subject: Re: Running lint-java during PR builds?
>
> I'm fine with it. I tried to use the existing checkstyle sbt plugin
> (trying to fix SPARK-22269), but it depends on an ancient version of
> checkstyle, and I don't know sbt enough to figure out how to hack
> classpaths and class loaders when applying rules, so gave up.
>
> On Mon, May 21, 2018 at 1:47 AM, Hyukjin Kwon <[hidden email]> wrote:
>> I am going to open an INFRA JIRA if there's no explicit objection in few
>> days.
>>
>> 2018-05-21 13:09 GMT+08:00 Hyukjin Kwon <[hidden email]>:
>>>
>>> I would like to revive this proposal. Travis CI. Shall we give this try?
>>> I
>>> think it's worth trying it.
>>>
>>> 2016-11-17 3:50 GMT+08:00 Dongjoon Hyun <[hidden email]>:
>>>>
>>>> Hi, Marcelo and Ryan.
>>>>
>>>> That was the main purpose of my proposal about Travis.CI.
>>>> IMO, that is the only way to achieve that without any harmful
>>>> side-effect
>>>> on Jenkins infra.
>>>>
>>>> Spark is already ready for that. Like AppVoyer, if one of you files an
>>>> INFRA jira issue to enable that, they will turn on that. Then, we can
>>>> try it
>>>> and see the result. Also, you can turn off easily again if you don't
>>>> want.
>>>>
>>>> Without this, we will consume more community efforts. For example, we
>>>> merged lint-java error fix PR seven hours ago, but the master branch
>>>> still
>>>> has one lint-java error.
>>>>
>>>> https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319
>>>>
>>>> Actually, I've been monitoring the history here. (It's synced every 30
>>>> minutes.)
>>>>
>>>> https://travis-ci.org/dongjoon-hyun/spark/builds
>>>>
>>>> Could we give a change to this?
>>>>
>>>> Bests,
>>>> Dongjoon.
>>>>
>>>> On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu"
>>>> <[hidden email]> wrote:
>>>> > I remember it's because you need to run `mvn install` before running
>>>> > lint-java if the maven cache is empty, and `mvn install` is pretty
>>>> > heavy.
>>>> >
>>>> > On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]>
>>>> > wrote:
>>>> >
>>>> > > Hey all,
>>>> > >
>>>> > > Is there a reason why lint-java is not run during PR builds? I see
>>>> > > it
>>>> > > seems to be maven-only, is it really expensive to run after an sbt
>>>> > > build?
>>>> > >
>>>> > > I see a lot of PRs coming in to fix Java style issues, and those all
>>>> > > seem a little unnecessary. Either we're enforcing style checks or
>>>> > > we're not, and right now it seems we aren't.
>>>> > >
>>>> > > --
>>>> > > Marcelo
>>>> > >
>>>> > >
>>>> > > ---------------------------------------------------------------------
>>>> > > To unsubscribe e-mail: [hidden email]
>>>> > >
>>>> > >
>>>> >
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe e-mail: [hidden email]
>>>>
>>>
>>
>
>
>
> --
> Marcelo
>
> ---------------------------------------------------------------------
> To unsubscribe e-mail: [hidden email]
>



--
Marcelo

Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

rxin
Can we look into if there is a plugin for sbt that works and then we can put everything into one single builder?

On Mon, May 21, 2018 at 11:17 AM Dongjoon Hyun <[hidden email]> wrote:
Thank you for reconsidering this, Hyukjin. :)

Bests,
Dongjoon.


On Mon, May 21, 2018 at 9:20 AM, Marcelo Vanzin <[hidden email]> wrote:
Is there a way to trigger it conditionally? e.g. only if the diff
touches java files.

On Mon, May 21, 2018 at 9:17 AM, Felix Cheung <[hidden email]> wrote:
> One concern is with the volume of test runs on Travis.
>
> In ASF projects Travis could get significantly
> backed up since - if I recall - all of ASF shares one queue.
>
> At the number of PRs Spark has this could be a big issue.
>
>
> ________________________________
> From: Marcelo Vanzin <[hidden email]>
> Sent: Monday, May 21, 2018 9:08:28 AM
> To: Hyukjin Kwon
> Cc: Dongjoon Hyun; dev
> Subject: Re: Running lint-java during PR builds?
>
> I'm fine with it. I tried to use the existing checkstyle sbt plugin
> (trying to fix SPARK-22269), but it depends on an ancient version of
> checkstyle, and I don't know sbt enough to figure out how to hack
> classpaths and class loaders when applying rules, so gave up.
>
> On Mon, May 21, 2018 at 1:47 AM, Hyukjin Kwon <[hidden email]> wrote:
>> I am going to open an INFRA JIRA if there's no explicit objection in few
>> days.
>>
>> 2018-05-21 13:09 GMT+08:00 Hyukjin Kwon <[hidden email]>:
>>>
>>> I would like to revive this proposal. Travis CI. Shall we give this try?
>>> I
>>> think it's worth trying it.
>>>
>>> 2016-11-17 3:50 GMT+08:00 Dongjoon Hyun <[hidden email]>:
>>>>
>>>> Hi, Marcelo and Ryan.
>>>>
>>>> That was the main purpose of my proposal about Travis.CI.
>>>> IMO, that is the only way to achieve that without any harmful
>>>> side-effect
>>>> on Jenkins infra.
>>>>
>>>> Spark is already ready for that. Like AppVoyer, if one of you files an
>>>> INFRA jira issue to enable that, they will turn on that. Then, we can
>>>> try it
>>>> and see the result. Also, you can turn off easily again if you don't
>>>> want.
>>>>
>>>> Without this, we will consume more community efforts. For example, we
>>>> merged lint-java error fix PR seven hours ago, but the master branch
>>>> still
>>>> has one lint-java error.
>>>>
>>>> https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319
>>>>
>>>> Actually, I've been monitoring the history here. (It's synced every 30
>>>> minutes.)
>>>>
>>>> https://travis-ci.org/dongjoon-hyun/spark/builds
>>>>
>>>> Could we give a change to this?
>>>>
>>>> Bests,
>>>> Dongjoon.
>>>>
>>>> On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu"
>>>> <[hidden email]> wrote:
>>>> > I remember it's because you need to run `mvn install` before running
>>>> > lint-java if the maven cache is empty, and `mvn install` is pretty
>>>> > heavy.
>>>> >
>>>> > On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]>
>>>> > wrote:
>>>> >
>>>> > > Hey all,
>>>> > >
>>>> > > Is there a reason why lint-java is not run during PR builds? I see
>>>> > > it
>>>> > > seems to be maven-only, is it really expensive to run after an sbt
>>>> > > build?
>>>> > >
>>>> > > I see a lot of PRs coming in to fix Java style issues, and those all
>>>> > > seem a little unnecessary. Either we're enforcing style checks or
>>>> > > we're not, and right now it seems we aren't.
>>>> > >
>>>> > > --
>>>> > > Marcelo
>>>> > >
>>>> > >
>>>> > > ---------------------------------------------------------------------
>>>> > > To unsubscribe e-mail: [hidden email]
>>>> > >
>>>> > >
>>>> >
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe e-mail: [hidden email]
>>>>
>>>
>>
>
>
>
> --
> Marcelo
>
> ---------------------------------------------------------------------
> To unsubscribe e-mail: [hidden email]
>



--
Marcelo

Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Hyukjin Kwon
I opened a PR - https://github.com/apache/spark/pull/21399 to run it with SBT.

2018-05-22 2:18 GMT+08:00 Reynold Xin <[hidden email]>:
Can we look into if there is a plugin for sbt that works and then we can put everything into one single builder?

On Mon, May 21, 2018 at 11:17 AM Dongjoon Hyun <[hidden email]> wrote:
Thank you for reconsidering this, Hyukjin. :)

Bests,
Dongjoon.


On Mon, May 21, 2018 at 9:20 AM, Marcelo Vanzin <[hidden email]> wrote:
Is there a way to trigger it conditionally? e.g. only if the diff
touches java files.

On Mon, May 21, 2018 at 9:17 AM, Felix Cheung <[hidden email]> wrote:
> One concern is with the volume of test runs on Travis.
>
> In ASF projects Travis could get significantly
> backed up since - if I recall - all of ASF shares one queue.
>
> At the number of PRs Spark has this could be a big issue.
>
>
> ________________________________
> From: Marcelo Vanzin <[hidden email]>
> Sent: Monday, May 21, 2018 9:08:28 AM
> To: Hyukjin Kwon
> Cc: Dongjoon Hyun; dev
> Subject: Re: Running lint-java during PR builds?
>
> I'm fine with it. I tried to use the existing checkstyle sbt plugin
> (trying to fix SPARK-22269), but it depends on an ancient version of
> checkstyle, and I don't know sbt enough to figure out how to hack
> classpaths and class loaders when applying rules, so gave up.
>
> On Mon, May 21, 2018 at 1:47 AM, Hyukjin Kwon <[hidden email]> wrote:
>> I am going to open an INFRA JIRA if there's no explicit objection in few
>> days.
>>
>> 2018-05-21 13:09 GMT+08:00 Hyukjin Kwon <[hidden email]>:
>>>
>>> I would like to revive this proposal. Travis CI. Shall we give this try?
>>> I
>>> think it's worth trying it.
>>>
>>> 2016-11-17 3:50 GMT+08:00 Dongjoon Hyun <[hidden email]>:
>>>>
>>>> Hi, Marcelo and Ryan.
>>>>
>>>> That was the main purpose of my proposal about Travis.CI.
>>>> IMO, that is the only way to achieve that without any harmful
>>>> side-effect
>>>> on Jenkins infra.
>>>>
>>>> Spark is already ready for that. Like AppVoyer, if one of you files an
>>>> INFRA jira issue to enable that, they will turn on that. Then, we can
>>>> try it
>>>> and see the result. Also, you can turn off easily again if you don't
>>>> want.
>>>>
>>>> Without this, we will consume more community efforts. For example, we
>>>> merged lint-java error fix PR seven hours ago, but the master branch
>>>> still
>>>> has one lint-java error.
>>>>
>>>> https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319
>>>>
>>>> Actually, I've been monitoring the history here. (It's synced every 30
>>>> minutes.)
>>>>
>>>> https://travis-ci.org/dongjoon-hyun/spark/builds
>>>>
>>>> Could we give a change to this?
>>>>
>>>> Bests,
>>>> Dongjoon.
>>>>
>>>> On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu"
>>>> <[hidden email]> wrote:
>>>> > I remember it's because you need to run `mvn install` before running
>>>> > lint-java if the maven cache is empty, and `mvn install` is pretty
>>>> > heavy.
>>>> >
>>>> > On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]>
>>>> > wrote:
>>>> >
>>>> > > Hey all,
>>>> > >
>>>> > > Is there a reason why lint-java is not run during PR builds? I see
>>>> > > it
>>>> > > seems to be maven-only, is it really expensive to run after an sbt
>>>> > > build?
>>>> > >
>>>> > > I see a lot of PRs coming in to fix Java style issues, and those all
>>>> > > seem a little unnecessary. Either we're enforcing style checks or
>>>> > > we're not, and right now it seems we aren't.
>>>> > >
>>>> > > --
>>>> > > Marcelo
>>>> > >
>>>> > >
>>>> > > ---------------------------------------------------------------------
>>>> > > To unsubscribe e-mail: [hidden email]
>>>> > >
>>>> > >
>>>> >
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe e-mail: [hidden email]
>>>>
>>>
>>
>
>
>
> --
> Marcelo
>
> ---------------------------------------------------------------------
> To unsubscribe e-mail: [hidden email]
>



--
Marcelo


Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Steve Loughran
In reply to this post by Marcelo Vanzin


> On 21 May 2018, at 17:20, Marcelo Vanzin <[hidden email]> wrote:
>
> Is there a way to trigger it conditionally? e.g. only if the diff
> touches java files.
>

what about adding it as another command which could be added alongside "jenkins test this please", something like "lint this please"

> On Mon, May 21, 2018 at 9:17 AM, Felix Cheung <[hidden email]> wrote:
>> One concern is with the volume of test runs on Travis.
>>
>> In ASF projects Travis could get significantly
>> backed up since - if I recall - all of ASF shares one queue.
>>
>> At the number of PRs Spark has this could be a big issue.
>>
>>
>> ________________________________
>> From: Marcelo Vanzin <[hidden email]>
>> Sent: Monday, May 21, 2018 9:08:28 AM
>> To: Hyukjin Kwon
>> Cc: Dongjoon Hyun; dev
>> Subject: Re: Running lint-java during PR builds?
>>
>> I'm fine with it. I tried to use the existing checkstyle sbt plugin
>> (trying to fix SPARK-22269), but it depends on an ancient version of
>> checkstyle, and I don't know sbt enough to figure out how to hack
>> classpaths and class loaders when applying rules, so gave up.
>>
>> On Mon, May 21, 2018 at 1:47 AM, Hyukjin Kwon <[hidden email]> wrote:
>>> I am going to open an INFRA JIRA if there's no explicit objection in few
>>> days.
>>>
>>> 2018-05-21 13:09 GMT+08:00 Hyukjin Kwon <[hidden email]>:
>>>>
>>>> I would like to revive this proposal. Travis CI. Shall we give this try?
>>>> I
>>>> think it's worth trying it.
>>>>
>>>> 2016-11-17 3:50 GMT+08:00 Dongjoon Hyun <[hidden email]>:
>>>>>
>>>>> Hi, Marcelo and Ryan.
>>>>>
>>>>> That was the main purpose of my proposal about Travis.CI.
>>>>> IMO, that is the only way to achieve that without any harmful
>>>>> side-effect
>>>>> on Jenkins infra.
>>>>>
>>>>> Spark is already ready for that. Like AppVoyer, if one of you files an
>>>>> INFRA jira issue to enable that, they will turn on that. Then, we can
>>>>> try it
>>>>> and see the result. Also, you can turn off easily again if you don't
>>>>> want.
>>>>>
>>>>> Without this, we will consume more community efforts. For example, we
>>>>> merged lint-java error fix PR seven hours ago, but the master branch
>>>>> still
>>>>> has one lint-java error.
>>>>>
>>>>> https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319
>>>>>
>>>>> Actually, I've been monitoring the history here. (It's synced every 30
>>>>> minutes.)
>>>>>
>>>>> https://travis-ci.org/dongjoon-hyun/spark/builds
>>>>>
>>>>> Could we give a change to this?
>>>>>
>>>>> Bests,
>>>>> Dongjoon.
>>>>>
>>>>> On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu"
>>>>> <[hidden email]> wrote:
>>>>>> I remember it's because you need to run `mvn install` before running
>>>>>> lint-java if the maven cache is empty, and `mvn install` is pretty
>>>>>> heavy.
>>>>>>
>>>>>> On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]>
>>>>>> wrote:
>>>>>>
>>>>>>> Hey all,
>>>>>>>
>>>>>>> Is there a reason why lint-java is not run during PR builds? I see
>>>>>>> it
>>>>>>> seems to be maven-only, is it really expensive to run after an sbt
>>>>>>> build?
>>>>>>>
>>>>>>> I see a lot of PRs coming in to fix Java style issues, and those all
>>>>>>> seem a little unnecessary. Either we're enforcing style checks or
>>>>>>> we're not, and right now it seems we aren't.
>>>>>>>
>>>>>>> --
>>>>>>> Marcelo
>>>>>>>
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe e-mail: [hidden email]
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe e-mail: [hidden email]
>>>>>
>>>>
>>>
>>
>>
>>
>> --
>> Marcelo
>>
>> ---------------------------------------------------------------------
>> To unsubscribe e-mail: [hidden email]
>>
>
>
>
> --
> Marcelo
>
> ---------------------------------------------------------------------
> To unsubscribe e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: Running lint-java during PR builds?

Hyukjin Kwon
Ah, now it's merged so that it can be ran via SBT and it runs when there are some changes in .java file only.

On Mon, 28 May 2018, 6:23 pm Steve Loughran, <[hidden email]> wrote:


> On 21 May 2018, at 17:20, Marcelo Vanzin <[hidden email]> wrote:
>
> Is there a way to trigger it conditionally? e.g. only if the diff
> touches java files.
>

what about adding it as another command which could be added alongside "jenkins test this please", something like "lint this please"

> On Mon, May 21, 2018 at 9:17 AM, Felix Cheung <[hidden email]> wrote:
>> One concern is with the volume of test runs on Travis.
>>
>> In ASF projects Travis could get significantly
>> backed up since - if I recall - all of ASF shares one queue.
>>
>> At the number of PRs Spark has this could be a big issue.
>>
>>
>> ________________________________
>> From: Marcelo Vanzin <[hidden email]>
>> Sent: Monday, May 21, 2018 9:08:28 AM
>> To: Hyukjin Kwon
>> Cc: Dongjoon Hyun; dev
>> Subject: Re: Running lint-java during PR builds?
>>
>> I'm fine with it. I tried to use the existing checkstyle sbt plugin
>> (trying to fix SPARK-22269), but it depends on an ancient version of
>> checkstyle, and I don't know sbt enough to figure out how to hack
>> classpaths and class loaders when applying rules, so gave up.
>>
>> On Mon, May 21, 2018 at 1:47 AM, Hyukjin Kwon <[hidden email]> wrote:
>>> I am going to open an INFRA JIRA if there's no explicit objection in few
>>> days.
>>>
>>> 2018-05-21 13:09 GMT+08:00 Hyukjin Kwon <[hidden email]>:
>>>>
>>>> I would like to revive this proposal. Travis CI. Shall we give this try?
>>>> I
>>>> think it's worth trying it.
>>>>
>>>> 2016-11-17 3:50 GMT+08:00 Dongjoon Hyun <[hidden email]>:
>>>>>
>>>>> Hi, Marcelo and Ryan.
>>>>>
>>>>> That was the main purpose of my proposal about Travis.CI.
>>>>> IMO, that is the only way to achieve that without any harmful
>>>>> side-effect
>>>>> on Jenkins infra.
>>>>>
>>>>> Spark is already ready for that. Like AppVoyer, if one of you files an
>>>>> INFRA jira issue to enable that, they will turn on that. Then, we can
>>>>> try it
>>>>> and see the result. Also, you can turn off easily again if you don't
>>>>> want.
>>>>>
>>>>> Without this, we will consume more community efforts. For example, we
>>>>> merged lint-java error fix PR seven hours ago, but the master branch
>>>>> still
>>>>> has one lint-java error.
>>>>>
>>>>> https://travis-ci.org/dongjoon-hyun/spark/jobs/176351319
>>>>>
>>>>> Actually, I've been monitoring the history here. (It's synced every 30
>>>>> minutes.)
>>>>>
>>>>> https://travis-ci.org/dongjoon-hyun/spark/builds
>>>>>
>>>>> Could we give a change to this?
>>>>>
>>>>> Bests,
>>>>> Dongjoon.
>>>>>
>>>>> On 2016-11-15 13:40 (-0800), "Shixiong(Ryan) Zhu"
>>>>> <[hidden email]> wrote:
>>>>>> I remember it's because you need to run `mvn install` before running
>>>>>> lint-java if the maven cache is empty, and `mvn install` is pretty
>>>>>> heavy.
>>>>>>
>>>>>> On Tue, Nov 15, 2016 at 1:21 PM, Marcelo Vanzin <[hidden email]>
>>>>>> wrote:
>>>>>>
>>>>>>> Hey all,
>>>>>>>
>>>>>>> Is there a reason why lint-java is not run during PR builds? I see
>>>>>>> it
>>>>>>> seems to be maven-only, is it really expensive to run after an sbt
>>>>>>> build?
>>>>>>>
>>>>>>> I see a lot of PRs coming in to fix Java style issues, and those all
>>>>>>> seem a little unnecessary. Either we're enforcing style checks or
>>>>>>> we're not, and right now it seems we aren't.
>>>>>>>
>>>>>>> --
>>>>>>> Marcelo
>>>>>>>
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe e-mail: [hidden email]
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe e-mail: [hidden email]
>>>>>
>>>>
>>>
>>
>>
>>
>> --
>> Marcelo
>>
>> ---------------------------------------------------------------------
>> To unsubscribe e-mail: [hidden email]
>>
>
>
>
> --
> Marcelo
>
> ---------------------------------------------------------------------
> To unsubscribe e-mail: [hidden email]
>