You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@distributedlog.apache.org by xieliang <gi...@git.apache.org> on 2016/12/13 10:14:53 UTC

[GitHub] incubator-distributedlog pull request #71: DL-140 : Fix distributedlog-core ...

GitHub user xieliang opened a pull request:

    https://github.com/apache/incubator-distributedlog/pull/71

    DL-140 : Fix distributedlog-core findbug inconsistent synchronization warings

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/xieliang/incubator-distributedlog DL-140

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-distributedlog/pull/71.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #71
    
----
commit febd945ffa409d861d0f783a78831b88ae56c540
Author: xieliang <xi...@gmail.com>
Date:   2016-12-13T10:11:02Z

    Fix distributedlog-core findbug inconsistent synchronization warings

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog issue #71: DL-140 : Fix distributedlog-core findbug...

Posted by xieliang <gi...@git.apache.org>.
Github user xieliang commented on the issue:

    https://github.com/apache/incubator-distributedlog/pull/71
  
    I ran the findbug (3.0.1) binary on my local devbox. Those multi-thread warings exist even after importing the findbug exclude xml file manually,  what's the findbugs binary version at ci side, i guess it's obsolete...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog issue #71: DL-140 : Fix distributedlog-core findbug...

Posted by xieliang <gi...@git.apache.org>.
Github user xieliang commented on the issue:

    https://github.com/apache/incubator-distributedlog/pull/71
  
    Rebased.  
    "it might be worth having a jira to make the findbugs threshold 'LOW'. and in this jira, we need to fix those findbug warnings."  , i'll create a new jira.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog issue #71: DL-140 : Fix distributedlog-core findbug...

Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:

    https://github.com/apache/incubator-distributedlog/pull/71
  
    @franckcuny I passed the travis ci for both scala 2.10 and 2.11. it should be transient errors.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog pull request #71: DL-140 : Fix distributedlog-core ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-distributedlog/pull/71


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog issue #71: DL-140 : Fix distributedlog-core findbug...

Posted by xieliang <gi...@git.apache.org>.
Github user xieliang commented on the issue:

    https://github.com/apache/incubator-distributedlog/pull/71
  
    @sijie , in distributedlog-core pom.xml, if you add "<threshold>Low</threshold>" after "<excludeFilterFile>"  config item, you will see sth like this:
    [INFO] BugInstance size is 120
    [INFO] Error size is 0
    [INFO] Total bugs: 120
    
    Just like my result from findbugs gui view.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog issue #71: DL-140 : Fix distributedlog-core findbug...

Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:

    https://github.com/apache/incubator-distributedlog/pull/71
  
    @xieliang I think currently we are using 3.0.3 in the pom.xml file.
    
    `      
        <plugin>
            <groupId>org.codehaus.mojo</groupId>
            <artifactId>findbugs-maven-plugin</artifactId>
            <version>3.0.3</version>
          </plugin>
    `
    The travis ci runs "findbugs:check" for every pull request - "mvn --batch-mode clean package findbugs:check"
    
    is it the same command that you are running?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog issue #71: DL-140 : Fix distributedlog-core findbug...

Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:

    https://github.com/apache/incubator-distributedlog/pull/71
  
    @xieliang gotcha. it might be worth having a jira to make the findbugs threshold 'LOW'. and in this jira, we need to fix those findbug warnings.
    
    for this pr, I think it is good to fix synchronization issue. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog issue #71: DL-140 : Fix distributedlog-core findbug...

Posted by franckcuny <gi...@git.apache.org>.
Github user franckcuny commented on the issue:

    https://github.com/apache/incubator-distributedlog/pull/71
  
    @xieliang could you verify if the failure is due to your change ? Is the test passing on your local environment ?
    
    cc @sijie 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog issue #71: DL-140 : Fix distributedlog-core findbug...

Posted by xieliang <gi...@git.apache.org>.
Github user xieliang commented on the issue:

    https://github.com/apache/incubator-distributedlog/pull/71
  
    It's not the same thing. The travis ci runs with "findbugs-maven-plugin", and i ran with findbugs binary package. 
    (From http://gleclaire.github.io/findbugs-maven-plugin/index.html , i see the core findbugs version are all 3.0.1.)
    Let me finger out the diff later


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog issue #71: DL-140 : Fix distributedlog-core findbug...

Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:

    https://github.com/apache/incubator-distributedlog/pull/71
  
    @xieliang do you mind rebasing this pr to latest master? so it would trigger the checks again.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-distributedlog issue #71: DL-140 : Fix distributedlog-core findbug...

Posted by sijie <gi...@git.apache.org>.
Github user sijie commented on the issue:

    https://github.com/apache/incubator-distributedlog/pull/71
  
    @xieliang the change looks good. but I am interested to see how did you find this. I thought we already enable findbugs on ci jobs. I am wondering why didn't we detect the findbug warnings.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---