You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@druid.apache.org by Chi Cao Minh <ch...@imply.io> on 2019/08/20 23:56:06 UTC

Coveralls.io and LGTM.com GitHub integrations

Coveralls.io <http://coveralls.io/> and LGTM.com <http://lgtm.com/> GitHub integrations are now enabled to help us improve code quality, and you’ll start seeing updates on pull requests from the respective bots. Also, for code coverage, JaCoCo is now used instead of Cobertura.

Druid’s Coveralls.io <http://coveralls.io/> page:
https://coveralls.io/github/apache/incubator-druid <https://coveralls.io/github/apache/incubator-druid>

Druid’s LGTM.com <http://lgtm.com/> page:
https://lgtm.com/projects/g/apache/incubator-druid/ <https://lgtm.com/projects/g/apache/incubator-druid/>

If you have feedback on either of the new GitHub integrations, please reply to this thread.

Thanks,
Chi


Re: Coveralls.io and LGTM.com GitHub integrations

Posted by Chi Cao Minh <ch...@imply.io>.
I’ve put up a PR to disable Coveralls until it can be fixed to work reliably or a better alternative can be found. Apologies for the disruption.
https://github.com/apache/incubator-druid/pull/8382

Thanks,
Chi


> On Aug 23, 2019, at 1:26 AM, Gian Merlino <gi...@apache.org> wrote:
> 
> Dave, I merged your patch (#8352) and also the other one I mentioned
> (#8368), since the errors seem spurious currently, so I don't think it
> makes sense to hold up PRs based on them.
> 
> On Fri, Aug 23, 2019 at 12:36 AM David Lim <da...@apache.org> wrote:
> 
>> Similar thing happened on
>> https://github.com/apache/incubator-druid/pull/8352 - the patch basically
>> adds a try/catch around a log statement and coverage decreased by 9.1%. I
>> tried restarting Travis to see if it would rerun Coveralls but it does not.
>> 
>> On Fri, Aug 23, 2019 at 1:31 AM Gian Merlino <gi...@apache.org> wrote:
>> 
>>> FYI, something seems wrong with Coveralls, e.g.
>>> 
>> https://github.com/apache/incubator-druid/pull/8368#issuecomment-524204050
>>> .
>>> 
>>> I'm not sure if https://github.com/apache/incubator-druid/pull/8374
>> fixes
>>> it or not; I think I saw that patch as a parent of the commit from #8368,
>>> even after the messed-up Coveralls result.
>>> 
>>> 
>>> On Tue, Aug 20, 2019 at 11:35 PM Gian Merlino <gi...@apache.org> wrote:
>>> 
>>>> Thanks, Chi!
>>>> 
>>>> I haven't had a chance to experience yet how these tools work with PRs,
>>>> but will keep a look out for how useful they seem. A quick scan through
>>> the
>>>> LGTM report of master shows that it found some real issues and isn't
>> too
>>>> noisy. There's a spread of false positives (it doesn't like loop
>>>> unrolling), borderline cases, and legitimate issues that would be great
>>> to
>>>> fix.
>>>> 
>>>> On Tue, Aug 20, 2019 at 4:56 PM Chi Cao Minh <ch...@imply.io>
>>> wrote:
>>>> 
>>>>> Coveralls.io <http://coveralls.io/> and LGTM.com <http://lgtm.com/>
>>>>> GitHub integrations are now enabled to help us improve code quality,
>> and
>>>>> you’ll start seeing updates on pull requests from the respective bots.
>>>>> Also, for code coverage, JaCoCo is now used instead of Cobertura.
>>>>> 
>>>>> Druid’s Coveralls.io <http://coveralls.io/> page:
>>>>> https://coveralls.io/github/apache/incubator-druid <
>>>>> https://coveralls.io/github/apache/incubator-druid>
>>>>> 
>>>>> Druid’s LGTM.com <http://lgtm.com/> page:
>>>>> https://lgtm.com/projects/g/apache/incubator-druid/ <
>>>>> https://lgtm.com/projects/g/apache/incubator-druid/>
>>>>> 
>>>>> If you have feedback on either of the new GitHub integrations, please
>>>>> reply to this thread.
>>>>> 
>>>>> Thanks,
>>>>> Chi
>>>>> 
>>>>> 
>>> 
>> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
For additional commands, e-mail: dev-help@druid.apache.org


Re: Coveralls.io and LGTM.com GitHub integrations

Posted by Gian Merlino <gi...@apache.org>.
Dave, I merged your patch (#8352) and also the other one I mentioned
(#8368), since the errors seem spurious currently, so I don't think it
makes sense to hold up PRs based on them.

On Fri, Aug 23, 2019 at 12:36 AM David Lim <da...@apache.org> wrote:

> Similar thing happened on
> https://github.com/apache/incubator-druid/pull/8352 - the patch basically
> adds a try/catch around a log statement and coverage decreased by 9.1%. I
> tried restarting Travis to see if it would rerun Coveralls but it does not.
>
> On Fri, Aug 23, 2019 at 1:31 AM Gian Merlino <gi...@apache.org> wrote:
>
> > FYI, something seems wrong with Coveralls, e.g.
> >
> https://github.com/apache/incubator-druid/pull/8368#issuecomment-524204050
> > .
> >
> > I'm not sure if https://github.com/apache/incubator-druid/pull/8374
> fixes
> > it or not; I think I saw that patch as a parent of the commit from #8368,
> > even after the messed-up Coveralls result.
> >
> >
> > On Tue, Aug 20, 2019 at 11:35 PM Gian Merlino <gi...@apache.org> wrote:
> >
> > > Thanks, Chi!
> > >
> > > I haven't had a chance to experience yet how these tools work with PRs,
> > > but will keep a look out for how useful they seem. A quick scan through
> > the
> > > LGTM report of master shows that it found some real issues and isn't
> too
> > > noisy. There's a spread of false positives (it doesn't like loop
> > > unrolling), borderline cases, and legitimate issues that would be great
> > to
> > > fix.
> > >
> > > On Tue, Aug 20, 2019 at 4:56 PM Chi Cao Minh <ch...@imply.io>
> > wrote:
> > >
> > >> Coveralls.io <http://coveralls.io/> and LGTM.com <http://lgtm.com/>
> > >> GitHub integrations are now enabled to help us improve code quality,
> and
> > >> you’ll start seeing updates on pull requests from the respective bots.
> > >> Also, for code coverage, JaCoCo is now used instead of Cobertura.
> > >>
> > >> Druid’s Coveralls.io <http://coveralls.io/> page:
> > >> https://coveralls.io/github/apache/incubator-druid <
> > >> https://coveralls.io/github/apache/incubator-druid>
> > >>
> > >> Druid’s LGTM.com <http://lgtm.com/> page:
> > >> https://lgtm.com/projects/g/apache/incubator-druid/ <
> > >> https://lgtm.com/projects/g/apache/incubator-druid/>
> > >>
> > >> If you have feedback on either of the new GitHub integrations, please
> > >> reply to this thread.
> > >>
> > >> Thanks,
> > >> Chi
> > >>
> > >>
> >
>

Re: Coveralls.io and LGTM.com GitHub integrations

Posted by Gian Merlino <gi...@apache.org>.
The Coveralls badge on our README (https://github.com/apache/incubator-druid)
is also annoying, because it's red, and red looks bad :)

On Fri, Aug 23, 2019 at 12:36 AM David Lim <da...@apache.org> wrote:

> Similar thing happened on
> https://github.com/apache/incubator-druid/pull/8352 - the patch basically
> adds a try/catch around a log statement and coverage decreased by 9.1%. I
> tried restarting Travis to see if it would rerun Coveralls but it does not.
>
> On Fri, Aug 23, 2019 at 1:31 AM Gian Merlino <gi...@apache.org> wrote:
>
> > FYI, something seems wrong with Coveralls, e.g.
> >
> https://github.com/apache/incubator-druid/pull/8368#issuecomment-524204050
> > .
> >
> > I'm not sure if https://github.com/apache/incubator-druid/pull/8374
> fixes
> > it or not; I think I saw that patch as a parent of the commit from #8368,
> > even after the messed-up Coveralls result.
> >
> >
> > On Tue, Aug 20, 2019 at 11:35 PM Gian Merlino <gi...@apache.org> wrote:
> >
> > > Thanks, Chi!
> > >
> > > I haven't had a chance to experience yet how these tools work with PRs,
> > > but will keep a look out for how useful they seem. A quick scan through
> > the
> > > LGTM report of master shows that it found some real issues and isn't
> too
> > > noisy. There's a spread of false positives (it doesn't like loop
> > > unrolling), borderline cases, and legitimate issues that would be great
> > to
> > > fix.
> > >
> > > On Tue, Aug 20, 2019 at 4:56 PM Chi Cao Minh <ch...@imply.io>
> > wrote:
> > >
> > >> Coveralls.io <http://coveralls.io/> and LGTM.com <http://lgtm.com/>
> > >> GitHub integrations are now enabled to help us improve code quality,
> and
> > >> you’ll start seeing updates on pull requests from the respective bots.
> > >> Also, for code coverage, JaCoCo is now used instead of Cobertura.
> > >>
> > >> Druid’s Coveralls.io <http://coveralls.io/> page:
> > >> https://coveralls.io/github/apache/incubator-druid <
> > >> https://coveralls.io/github/apache/incubator-druid>
> > >>
> > >> Druid’s LGTM.com <http://lgtm.com/> page:
> > >> https://lgtm.com/projects/g/apache/incubator-druid/ <
> > >> https://lgtm.com/projects/g/apache/incubator-druid/>
> > >>
> > >> If you have feedback on either of the new GitHub integrations, please
> > >> reply to this thread.
> > >>
> > >> Thanks,
> > >> Chi
> > >>
> > >>
> >
>

Re: Coveralls.io and LGTM.com GitHub integrations

Posted by David Lim <da...@apache.org>.
Similar thing happened on
https://github.com/apache/incubator-druid/pull/8352 - the patch basically
adds a try/catch around a log statement and coverage decreased by 9.1%. I
tried restarting Travis to see if it would rerun Coveralls but it does not.

On Fri, Aug 23, 2019 at 1:31 AM Gian Merlino <gi...@apache.org> wrote:

> FYI, something seems wrong with Coveralls, e.g.
> https://github.com/apache/incubator-druid/pull/8368#issuecomment-524204050
> .
>
> I'm not sure if https://github.com/apache/incubator-druid/pull/8374 fixes
> it or not; I think I saw that patch as a parent of the commit from #8368,
> even after the messed-up Coveralls result.
>
>
> On Tue, Aug 20, 2019 at 11:35 PM Gian Merlino <gi...@apache.org> wrote:
>
> > Thanks, Chi!
> >
> > I haven't had a chance to experience yet how these tools work with PRs,
> > but will keep a look out for how useful they seem. A quick scan through
> the
> > LGTM report of master shows that it found some real issues and isn't too
> > noisy. There's a spread of false positives (it doesn't like loop
> > unrolling), borderline cases, and legitimate issues that would be great
> to
> > fix.
> >
> > On Tue, Aug 20, 2019 at 4:56 PM Chi Cao Minh <ch...@imply.io>
> wrote:
> >
> >> Coveralls.io <http://coveralls.io/> and LGTM.com <http://lgtm.com/>
> >> GitHub integrations are now enabled to help us improve code quality, and
> >> you’ll start seeing updates on pull requests from the respective bots.
> >> Also, for code coverage, JaCoCo is now used instead of Cobertura.
> >>
> >> Druid’s Coveralls.io <http://coveralls.io/> page:
> >> https://coveralls.io/github/apache/incubator-druid <
> >> https://coveralls.io/github/apache/incubator-druid>
> >>
> >> Druid’s LGTM.com <http://lgtm.com/> page:
> >> https://lgtm.com/projects/g/apache/incubator-druid/ <
> >> https://lgtm.com/projects/g/apache/incubator-druid/>
> >>
> >> If you have feedback on either of the new GitHub integrations, please
> >> reply to this thread.
> >>
> >> Thanks,
> >> Chi
> >>
> >>
>

Re: Coveralls.io and LGTM.com GitHub integrations

Posted by Gian Merlino <gi...@apache.org>.
FYI, something seems wrong with Coveralls, e.g.
https://github.com/apache/incubator-druid/pull/8368#issuecomment-524204050.

I'm not sure if https://github.com/apache/incubator-druid/pull/8374 fixes
it or not; I think I saw that patch as a parent of the commit from #8368,
even after the messed-up Coveralls result.


On Tue, Aug 20, 2019 at 11:35 PM Gian Merlino <gi...@apache.org> wrote:

> Thanks, Chi!
>
> I haven't had a chance to experience yet how these tools work with PRs,
> but will keep a look out for how useful they seem. A quick scan through the
> LGTM report of master shows that it found some real issues and isn't too
> noisy. There's a spread of false positives (it doesn't like loop
> unrolling), borderline cases, and legitimate issues that would be great to
> fix.
>
> On Tue, Aug 20, 2019 at 4:56 PM Chi Cao Minh <ch...@imply.io> wrote:
>
>> Coveralls.io <http://coveralls.io/> and LGTM.com <http://lgtm.com/>
>> GitHub integrations are now enabled to help us improve code quality, and
>> you’ll start seeing updates on pull requests from the respective bots.
>> Also, for code coverage, JaCoCo is now used instead of Cobertura.
>>
>> Druid’s Coveralls.io <http://coveralls.io/> page:
>> https://coveralls.io/github/apache/incubator-druid <
>> https://coveralls.io/github/apache/incubator-druid>
>>
>> Druid’s LGTM.com <http://lgtm.com/> page:
>> https://lgtm.com/projects/g/apache/incubator-druid/ <
>> https://lgtm.com/projects/g/apache/incubator-druid/>
>>
>> If you have feedback on either of the new GitHub integrations, please
>> reply to this thread.
>>
>> Thanks,
>> Chi
>>
>>

Re: Coveralls.io and LGTM.com GitHub integrations

Posted by Gian Merlino <gi...@apache.org>.
Thanks, Chi!

I haven't had a chance to experience yet how these tools work with PRs, but
will keep a look out for how useful they seem. A quick scan through the
LGTM report of master shows that it found some real issues and isn't too
noisy. There's a spread of false positives (it doesn't like loop
unrolling), borderline cases, and legitimate issues that would be great to
fix.

On Tue, Aug 20, 2019 at 4:56 PM Chi Cao Minh <ch...@imply.io> wrote:

> Coveralls.io <http://coveralls.io/> and LGTM.com <http://lgtm.com/>
> GitHub integrations are now enabled to help us improve code quality, and
> you’ll start seeing updates on pull requests from the respective bots.
> Also, for code coverage, JaCoCo is now used instead of Cobertura.
>
> Druid’s Coveralls.io <http://coveralls.io/> page:
> https://coveralls.io/github/apache/incubator-druid <
> https://coveralls.io/github/apache/incubator-druid>
>
> Druid’s LGTM.com <http://lgtm.com/> page:
> https://lgtm.com/projects/g/apache/incubator-druid/ <
> https://lgtm.com/projects/g/apache/incubator-druid/>
>
> If you have feedback on either of the new GitHub integrations, please
> reply to this thread.
>
> Thanks,
> Chi
>
>