Skip to content

Conversation

@pkumar-singh
Copy link
Member

@pkumar-singh pkumar-singh commented Apr 7, 2021

Motivation

Migrate bookkeeper to gradle spotbug plugin.
spotbugs is gradle as well as maven plugin which checks for obvious bugs in source code.
How to run
./gradlew spotbugsMain

Changes

  • Integrate gradle build with spotbugs plugin
  • Second commit is for fixing issues reported by spotbugs plugin

Test

Just to validate it's working.
I introduced following bug.

public void test() {
+
+        String[] field = {"a", "b", "c", "s", "e"};
+
+        //concatenates strings using + in a loop
+        String ip = "127.0.0.1";
+        String s = "";
+        for (int i = 0; i < field.length; ++i) {
+            s = s + field[i];
+        }
+
+        System.out.println(ip);
+
+    }

spotbugs plugin reported following violation.

<BugInstance type="SBSC_USE_STRINGBUFFER_CONCATENATION" priority="2" rank="18" abbrev="SBSC" category="PERFORMANCE" instanceHash="2aa8e4ebcc4cd86190fe3e7bd8a4913c" instanceOccurrenceNum="0" instanceOccurrenceMax="0">
    <ShortMessage>Method concatenates strings using + in a loop</ShortMessage>
    <LongMessage>org.apache.bookkeeper.meta.LongZkLedgerIdGenerator.test() concatenates strings using + in a loop</LongMessage>
    <Class classname="org.apache.bookkeeper.meta.LongZkLedgerIdGenerator" primary="true">
      <SourceLine classname="org.apache.bookkeeper.meta.LongZkLedgerIdGenerator" start="58" end="350" sourcefile="LongZkLedgerIdGenerator.java" sourcepath="org/apache/bookkeeper/meta/LongZkLedgerIdGenerator.java" relSourcepath="java/org/apache/bookkeeper/meta/LongZkLedgerIdGenerator.java">
        <Message>At LongZkLedgerIdGenerator.java:[lines 58-350]</Message>
      </SourceLine>
      <Message>In class org.apache.bookkeeper.meta.LongZkLedgerIdGenerator</Message>
    </Class>

Master Issue: #2640

@pkumar-singh pkumar-singh changed the title Merge internal gradle spotbugs ISSUE-2640: BP-43 Integrate spotbugs plugin with gradle Apr 7, 2021
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall +1
I left one comment about the version of spotbugs gradle vs Maven

@pkumar-singh pkumar-singh force-pushed the merge_internal_gradle_spotbugs branch from 08bf66d to bc5d374 Compare April 12, 2021 16:48
@pkumar-singh pkumar-singh requested a review from eolivelli April 12, 2021 16:53
@pkumar-singh pkumar-singh force-pushed the merge_internal_gradle_spotbugs branch from bc5d374 to 5883732 Compare April 13, 2021 19:21
Copy link
Contributor

@hsaputra hsaputra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@merlimat
Copy link
Contributor

@pkumar-singh You might need to rebase on master to get fixes for CI

@merlimat merlimat added this to the 4.14.0 milestone Apr 15, 2021
@hsaputra
Copy link
Contributor

The failed tests seems coming from timeouts and repeating fails in other PRs.
Since the change is related gradle changes, will merge this one.

@hsaputra hsaputra merged commit 44b1069 into apache:master Apr 16, 2021
@hsaputra hsaputra removed the type/bug label Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants