hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ted Yu <yuzhih...@gmail.com>
Subject Re: [DISCUSS] IntelliJ Code Analysis + Code Style
Date Mon, 19 Jun 2017 21:40:31 GMT
bq. we can apply them to both branch-1 and branch-2

Many bug fixes are still targeting 1.x releases. If the stylistic fixes go
to branch-1, would that make porting between 1.x releases and branch-1 more
cumbersome ?

What is the amount of changes for the stylistic fixes you deem applicable ?

Cheers

On Mon, Jun 19, 2017 at 2:30 PM, Mike Drob <mdrob@apache.org> wrote:

> Hi Devs,
>
> I recently ran the IntelliJ code analysis on the hbase project and saw that
> it had some interesting suggestions.
>
> A lot of them don't point to bugs, but really end up being more of a
> stylistic issues. The ones I want to discuss specifically are the ones it
> categorizes as java version issues. Here's the list:
>
> Java 5:
> * 'for' loop replaceable with 'foreach'
> * Unnecessary unboxing
> Java 7:
> * Explicit type can be replaced with <>
> * Identical 'catch' branch in 'try' statements
> * Possible heap pollution from vararg type
> Java 8:
> * Anonymous type can be replaced with lambda
> * Anonymous type can be replaced with method reference
> * Lambda can be replaced with method reference
> * Statement lambda can be replaced with expression lambda
>
> I think I read somewhere once that 'foreach' is not great for garbage
> collection because it creates an extra iterator, so we can maybe ignore
> that one. And I personally prefer statement lambdas (with curly braces) to
> expression lambdas (without) especially for long lines, so ignore that one
> too. But maybe address the rest?
>
> If we focus on only the Java 5/7 issues, then we can apply them to both
> branch-1 and branch-2. Not sure how much value that provides, as I don't
> have the historical context to estimate how long branch-1 will continue to
> live once branch-2 is out.
>
> The IDE can do the code changes automatically, but it will still be a lot
> of time to review, hence the discussion before I cavalierly go and file an
> issue and attach a patch. Also, I have no idea how to turn these into
> automated checks once we fix any of them, and that seems kind of important.
>
> IntelliJ has other checks that we can look at as well, but these were the
> ones that caught my eye first as easiest to fix and least likely to false
> positive.
>
> Thoughts?
>
> Mike
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message