You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@reef.apache.org by Sergiy Matusevych <se...@gmail.com> on 2020/02/21 06:12:30 UTC
Updating Checkstyle and fixing newly found warnings
Hey REEF team,
I've just posted a new PR that updates Checkstyle to a newer version and
fixes the warnings that the new version found in our code:
https://github.com/apache/reef/pull/1497
Does anyone wants to test and merge it into master?
Scott, would you like to look at it?
Once we merge that PR, I'll send the next one with the FindBugs update.
P.S. Also, are there any takers for the Parquet fix? That's issue
https://issues.apache.org/jira/projects/REEF/issues/REEF-2060
Cheers,
Sergiy.
Re: Updating Checkstyle and fixing newly found warnings
Posted by Dongjoon Hyun <do...@gmail.com>.
Got it. +1 for removal.
Bests,
Dongjoon.
On Mon, Feb 24, 2020 at 4:00 PM Sergiy Matusevych <
sergiy.matusevych@gmail.com> wrote:
> Well, as a *style* issue, it is not strictly required, of course, but the
> new CheckStyle views these `finals` in interfaces (and interfaces only) as
> redundant, hence the warning. to resolve it, we can either fix the code
> (and that's what I did in this PR), or disable the warning in the
> CheckStyle config. I personally am in favor of enabling as many checks as
> possible, but if you prefer the old code I can just disable this warning
> instead. WDYT?
>
> Thank you for looking at the PR!
> Sergiy.
>
>
> On Mon, Feb 24, 2020 at 11:06 AM Dongjoon Hyun <do...@gmail.com>
> wrote:
>
> > Hi, Sergiy.
> >
> > I took a look. Most of code changes are the removal of `final` keyword.
> Is
> > it required?
> >
> > Bests,
> > Dongjoon.
> >
> >
> > On Thu, Feb 20, 2020 at 10:13 PM Sergiy Matusevych <
> > sergiy.matusevych@gmail.com> wrote:
> >
> > > Hey REEF team,
> > >
> > > I've just posted a new PR that updates Checkstyle to a newer version
> and
> > > fixes the warnings that the new version found in our code:
> > >
> > > https://github.com/apache/reef/pull/1497
> > >
> > > Does anyone wants to test and merge it into master?
> > > Scott, would you like to look at it?
> > >
> > > Once we merge that PR, I'll send the next one with the FindBugs update.
> > >
> > > P.S. Also, are there any takers for the Parquet fix? That's issue
> > > https://issues.apache.org/jira/projects/REEF/issues/REEF-2060
> > >
> > > Cheers,
> > > Sergiy.
> > >
> >
>
Re: Updating Checkstyle and fixing newly found warnings
Posted by Sergiy Matusevych <se...@gmail.com>.
Well, as a *style* issue, it is not strictly required, of course, but the
new CheckStyle views these `finals` in interfaces (and interfaces only) as
redundant, hence the warning. to resolve it, we can either fix the code
(and that's what I did in this PR), or disable the warning in the
CheckStyle config. I personally am in favor of enabling as many checks as
possible, but if you prefer the old code I can just disable this warning
instead. WDYT?
Thank you for looking at the PR!
Sergiy.
On Mon, Feb 24, 2020 at 11:06 AM Dongjoon Hyun <do...@gmail.com>
wrote:
> Hi, Sergiy.
>
> I took a look. Most of code changes are the removal of `final` keyword. Is
> it required?
>
> Bests,
> Dongjoon.
>
>
> On Thu, Feb 20, 2020 at 10:13 PM Sergiy Matusevych <
> sergiy.matusevych@gmail.com> wrote:
>
> > Hey REEF team,
> >
> > I've just posted a new PR that updates Checkstyle to a newer version and
> > fixes the warnings that the new version found in our code:
> >
> > https://github.com/apache/reef/pull/1497
> >
> > Does anyone wants to test and merge it into master?
> > Scott, would you like to look at it?
> >
> > Once we merge that PR, I'll send the next one with the FindBugs update.
> >
> > P.S. Also, are there any takers for the Parquet fix? That's issue
> > https://issues.apache.org/jira/projects/REEF/issues/REEF-2060
> >
> > Cheers,
> > Sergiy.
> >
>
Re: Updating Checkstyle and fixing newly found warnings
Posted by Dongjoon Hyun <do...@gmail.com>.
Hi, Sergiy.
I took a look. Most of code changes are the removal of `final` keyword. Is
it required?
Bests,
Dongjoon.
On Thu, Feb 20, 2020 at 10:13 PM Sergiy Matusevych <
sergiy.matusevych@gmail.com> wrote:
> Hey REEF team,
>
> I've just posted a new PR that updates Checkstyle to a newer version and
> fixes the warnings that the new version found in our code:
>
> https://github.com/apache/reef/pull/1497
>
> Does anyone wants to test and merge it into master?
> Scott, would you like to look at it?
>
> Once we merge that PR, I'll send the next one with the FindBugs update.
>
> P.S. Also, are there any takers for the Parquet fix? That's issue
> https://issues.apache.org/jira/projects/REEF/issues/REEF-2060
>
> Cheers,
> Sergiy.
>