You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Kinga Marton via Review Board <no...@reviews.apache.org> on 2019/02/14 16:51:53 UTC

Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69988/
-----------------------------------------------------------

Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
-------

https://gleclaire.github.io/findbugs-maven-plugin/

Status: Since Findbugs is no longer maintained, please use Spotbugs which has a Maven plugin.

The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
It might worth to investigate this plugin.


Diffs
-----

  bin/test-patch-11-findbugs-diff c884daaa3 
  client/pom.xml f0f6a1b13 
  core/pom.xml b6c07d345 
  fluent-job/fluent-job-api/pom.xml f303b4583 
  fluent-job/pom.xml bb8861d59 
  pom.xml 93fffc791 
  sharelib/git/pom.xml 1c17e48ba 
  sharelib/oozie/pom.xml a53d335f9 
  sharelib/spark/pom.xml 76f69034e 
  tools/pom.xml 72a6c283a 


Diff: https://reviews.apache.org/r/69988/diff/1/


Testing
-------

Tested manually


Thanks,

Kinga Marton

Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

Posted by Andras Salamon <an...@melda.info>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69988/#review213080
-----------------------------------------------------------


Ship it!




Ship It!

- Andras Salamon


On Feb. 22, 2019, 8:30 a.m., Kinga Marton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69988/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2019, 8:30 a.m.)
> 
> 
> Review request for oozie and Andras Salamon.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> https://gleclaire.github.io/findbugs-maven-plugin/
> 
> Status: Since Findbugs is no longer maintained, please use Spotbugs which has a Maven plugin.
> 
> The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
> It might worth to investigate this plugin.
> 
> 
> Diffs
> -----
> 
>   bin/test-patch-11-findbugs-diff c884daaa3 
>   client/pom.xml f0f6a1b13 
>   core/pom.xml b6c07d345 
>   fluent-job/fluent-job-api/findbugs-filter.xml  
>   fluent-job/fluent-job-api/pom.xml f303b4583 
>   fluent-job/pom.xml bb8861d59 
>   pom.xml d817140f1 
>   sharelib/git/pom.xml 1c17e48ba 
>   sharelib/oozie/pom.xml a53d335f9 
>   sharelib/spark/pom.xml 76f69034e 
>   tools/pom.xml 72a6c283a 
> 
> 
> Diff: https://reviews.apache.org/r/69988/diff/4/
> 
> 
> Testing
> -------
> 
> Tested manually
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

Posted by Kinga Marton via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69988/
-----------------------------------------------------------

(Updated Feb. 22, 2019, 8:30 a.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
-------

https://gleclaire.github.io/findbugs-maven-plugin/

Status: Since Findbugs is no longer maintained, please use Spotbugs which has a Maven plugin.

The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
It might worth to investigate this plugin.


Diffs (updated)
-----

  bin/test-patch-11-findbugs-diff c884daaa3 
  client/pom.xml f0f6a1b13 
  core/pom.xml b6c07d345 
  fluent-job/fluent-job-api/findbugs-filter.xml  
  fluent-job/fluent-job-api/pom.xml f303b4583 
  fluent-job/pom.xml bb8861d59 
  pom.xml d817140f1 
  sharelib/git/pom.xml 1c17e48ba 
  sharelib/oozie/pom.xml a53d335f9 
  sharelib/spark/pom.xml 76f69034e 
  tools/pom.xml 72a6c283a 


Diff: https://reviews.apache.org/r/69988/diff/4/

Changes: https://reviews.apache.org/r/69988/diff/3-4/


Testing
-------

Tested manually


Thanks,

Kinga Marton

Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

Posted by Andras Salamon <an...@melda.info>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69988/#review213025
-----------------------------------------------------------




bin/test-patch-11-findbugs-diff
Line 118 (original), 119 (patched)
<https://reviews.apache.org/r/69988/#comment298879>

    I think the correct way to make both shellcheck and maven happy is the following:
    
    ${MVNPASSTHRU:+"${MVNPASSTHRU}"}
    
    Probably it's better to just ignore it.



bin/test-patch-11-findbugs-diff
Lines 186-187 (original), 187-188 (patched)
<https://reviews.apache.org/r/69988/#comment298875>

    Shellcheck gives the following error:
    
    SC2128: Expanding an array without an index only gives the first element.
    
    Why do we need an array here? Shouldn't we use a simple
    
    md5Content="$(md5sum "${DIFF_DIR}/${FINDBUGS_JAR}")"
    
    instead?



bin/test-patch-11-findbugs-diff
Line 202 (original), 203 (patched)
<https://reviews.apache.org/r/69988/#comment298876>

    Shellcheck warning is correct, moreover it is not even a valid command:
    
    $ echo "aaa" < cat /tmp/alma.txt
    -bash: cat: No such file or directory
    
    The correct syntax:
    
    $ echo "aaa $(cat /tmp/alma.txt)"    
    aaa alma
    
    Or we can just print out a simple error message without printing out the value of the md5. It's not that helpful information.



bin/test-patch-11-findbugs-diff
Line 204 (original), 205 (patched)
<https://reviews.apache.org/r/69988/#comment298877>

    Just like above, this is an invalid command.



bin/test-patch-11-findbugs-diff
Line 281 (original), 282 (patched)
<https://reviews.apache.org/r/69988/#comment298878>

    Let's just ignore the shellcheck warning here, this is too complex.


- Andras Salamon


On Feb. 20, 2019, 5:47 p.m., Kinga Marton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69988/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2019, 5:47 p.m.)
> 
> 
> Review request for oozie and Andras Salamon.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> https://gleclaire.github.io/findbugs-maven-plugin/
> 
> Status: Since Findbugs is no longer maintained, please use Spotbugs which has a Maven plugin.
> 
> The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
> It might worth to investigate this plugin.
> 
> 
> Diffs
> -----
> 
>   bin/test-patch-11-findbugs-diff c884daaa3 
>   client/pom.xml f0f6a1b13 
>   core/pom.xml b6c07d345 
>   fluent-job/fluent-job-api/findbugs-filter.xml  
>   fluent-job/fluent-job-api/pom.xml f303b4583 
>   fluent-job/pom.xml bb8861d59 
>   pom.xml d817140f1 
>   sharelib/git/pom.xml 1c17e48ba 
>   sharelib/oozie/pom.xml a53d335f9 
>   sharelib/spark/pom.xml 76f69034e 
>   tools/pom.xml 72a6c283a 
> 
> 
> Diff: https://reviews.apache.org/r/69988/diff/3/
> 
> 
> Testing
> -------
> 
> Tested manually
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

Posted by Kinga Marton via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69988/
-----------------------------------------------------------

(Updated Feb. 20, 2019, 5:47 p.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
-------

https://gleclaire.github.io/findbugs-maven-plugin/

Status: Since Findbugs is no longer maintained, please use Spotbugs which has a Maven plugin.

The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
It might worth to investigate this plugin.


Diffs (updated)
-----

  bin/test-patch-11-findbugs-diff c884daaa3 
  client/pom.xml f0f6a1b13 
  core/pom.xml b6c07d345 
  fluent-job/fluent-job-api/findbugs-filter.xml  
  fluent-job/fluent-job-api/pom.xml f303b4583 
  fluent-job/pom.xml bb8861d59 
  pom.xml d817140f1 
  sharelib/git/pom.xml 1c17e48ba 
  sharelib/oozie/pom.xml a53d335f9 
  sharelib/spark/pom.xml 76f69034e 
  tools/pom.xml 72a6c283a 


Diff: https://reviews.apache.org/r/69988/diff/3/

Changes: https://reviews.apache.org/r/69988/diff/2-3/


Testing
-------

Tested manually


Thanks,

Kinga Marton

Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

Posted by Kinga Marton via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69988/
-----------------------------------------------------------

(Updated Feb. 20, 2019, 2:20 p.m.)


Review request for oozie and Andras Salamon.


Repository: oozie-git


Description
-------

https://gleclaire.github.io/findbugs-maven-plugin/

Status: Since Findbugs is no longer maintained, please use Spotbugs which has a Maven plugin.

The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
It might worth to investigate this plugin.


Diffs (updated)
-----

  bin/test-patch-11-findbugs-diff c884daaa3 
  client/pom.xml f0f6a1b13 
  core/pom.xml b6c07d345 
  fluent-job/fluent-job-api/findbugs-filter.xml  
  fluent-job/fluent-job-api/pom.xml f303b4583 
  fluent-job/pom.xml bb8861d59 
  pom.xml d817140f1 
  sharelib/git/pom.xml 1c17e48ba 
  sharelib/oozie/pom.xml a53d335f9 
  sharelib/spark/pom.xml 76f69034e 
  tools/pom.xml 72a6c283a 


Diff: https://reviews.apache.org/r/69988/diff/2/

Changes: https://reviews.apache.org/r/69988/diff/1-2/


Testing
-------

Tested manually


Thanks,

Kinga Marton

Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

Posted by Kinga Marton via Review Board <no...@reviews.apache.org>.

> On Feb. 15, 2019, 10:02 a.m., Andras Salamon wrote:
> > bin/test-patch-11-findbugs-diff
> > Line 1 (original), 1 (patched)
> > <https://reviews.apache.org/r/69988/diff/1/?file=2125364#file2125364line1>
> >
> >     Can you please check the script with shellcheck

I have left a few warnings, because I wasn't sure that the shellcheck was right.


> On Feb. 15, 2019, 10:02 a.m., Andras Salamon wrote:
> > bin/test-patch-11-findbugs-diff
> > Line 275 (original), 275 (patched)
> > <https://reviews.apache.org/r/69988/diff/1/?file=2125364#file2125364line275>
> >
> >     Can we change the name to spotbugs-new.xml? Or maybe it's hardwired somewhere else?

This findbugs-new.xml is generated by findbugs-diff-0.1.0-all.jar, and unfortunately this value is hard-coded: https://github.com/AndersDJohnson/findbugs-diff/search?q=findbugs-new.xml&unscoped_q=findbugs-new.xml


> On Feb. 15, 2019, 10:02 a.m., Andras Salamon wrote:
> > fluent-job/fluent-job-api/pom.xml
> > Line 80 (original), 80 (patched)
> > <https://reviews.apache.org/r/69988/diff/1/?file=2125367#file2125367line80>
> >
> >     Should we keep this filename?

I have renamed it, but we should convert the content of the file into annotations. I will create an issue for it.


> On Feb. 15, 2019, 10:02 a.m., Andras Salamon wrote:
> > pom.xml
> > Line 1865 (original), 1871 (patched)
> > <https://reviews.apache.org/r/69988/diff/1/?file=2125369#file2125369line1872>
> >
> >     Why is it called findbug? Compatibility reasons?

findbug is a general name, even if the tool behind the check is changed, what we are actually doing is searching for bugs, so I think that this id is a valid one enem if now the spotbugs is doing the job.


- Kinga


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69988/#review212832
-----------------------------------------------------------


On Feb. 20, 2019, 2:20 p.m., Kinga Marton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69988/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2019, 2:20 p.m.)
> 
> 
> Review request for oozie and Andras Salamon.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> https://gleclaire.github.io/findbugs-maven-plugin/
> 
> Status: Since Findbugs is no longer maintained, please use Spotbugs which has a Maven plugin.
> 
> The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
> It might worth to investigate this plugin.
> 
> 
> Diffs
> -----
> 
>   bin/test-patch-11-findbugs-diff c884daaa3 
>   client/pom.xml f0f6a1b13 
>   core/pom.xml b6c07d345 
>   fluent-job/fluent-job-api/findbugs-filter.xml  
>   fluent-job/fluent-job-api/pom.xml f303b4583 
>   fluent-job/pom.xml bb8861d59 
>   pom.xml d817140f1 
>   sharelib/git/pom.xml 1c17e48ba 
>   sharelib/oozie/pom.xml a53d335f9 
>   sharelib/spark/pom.xml 76f69034e 
>   tools/pom.xml 72a6c283a 
> 
> 
> Diff: https://reviews.apache.org/r/69988/diff/2/
> 
> 
> Testing
> -------
> 
> Tested manually
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

Posted by Andras Salamon <an...@melda.info>.

> On Feb. 15, 2019, 10:02 a.m., Andras Salamon wrote:
> > fluent-job/fluent-job-api/pom.xml
> > Line 80 (original), 80 (patched)
> > <https://reviews.apache.org/r/69988/diff/1/?file=2125367#file2125367line80>
> >
> >     Should we keep this filename?
> 
> Kinga Marton wrote:
>     I have renamed it, but we should convert the content of the file into annotations. I will create an issue for it.

That would be great, but we generate those files so it might be difficult.


- Andras


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69988/#review212832
-----------------------------------------------------------


On Feb. 20, 2019, 2:20 p.m., Kinga Marton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69988/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2019, 2:20 p.m.)
> 
> 
> Review request for oozie and Andras Salamon.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> https://gleclaire.github.io/findbugs-maven-plugin/
> 
> Status: Since Findbugs is no longer maintained, please use Spotbugs which has a Maven plugin.
> 
> The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
> It might worth to investigate this plugin.
> 
> 
> Diffs
> -----
> 
>   bin/test-patch-11-findbugs-diff c884daaa3 
>   client/pom.xml f0f6a1b13 
>   core/pom.xml b6c07d345 
>   fluent-job/fluent-job-api/findbugs-filter.xml  
>   fluent-job/fluent-job-api/pom.xml f303b4583 
>   fluent-job/pom.xml bb8861d59 
>   pom.xml d817140f1 
>   sharelib/git/pom.xml 1c17e48ba 
>   sharelib/oozie/pom.xml a53d335f9 
>   sharelib/spark/pom.xml 76f69034e 
>   tools/pom.xml 72a6c283a 
> 
> 
> Diff: https://reviews.apache.org/r/69988/diff/2/
> 
> 
> Testing
> -------
> 
> Tested manually
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Re: Review Request 69988: OOZIE-3395 Findbugs is no longer maintained

Posted by Andras Salamon <an...@melda.info>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69988/#review212832
-----------------------------------------------------------




bin/test-patch-11-findbugs-diff
Line 1 (original), 1 (patched)
<https://reviews.apache.org/r/69988/#comment298747>

    Can you please check the script with shellcheck



bin/test-patch-11-findbugs-diff
Lines 28-29 (original), 28-29 (patched)
<https://reviews.apache.org/r/69988/#comment298748>

    If we change the order of the variables, we don't need to enter the jar name twice.



bin/test-patch-11-findbugs-diff
Line 238 (original), 238 (patched)
<https://reviews.apache.org/r/69988/#comment298749>

    Couldn't we use SPOTBUGS_XML_NAME here?



bin/test-patch-11-findbugs-diff
Line 275 (original), 275 (patched)
<https://reviews.apache.org/r/69988/#comment298753>

    Can we change the name to spotbugs-new.xml? Or maybe it's hardwired somewhere else?



bin/test-patch-11-findbugs-diff
Line 331 (original), 331 (patched)
<https://reviews.apache.org/r/69988/#comment298754>

    Even if we don't change the name, we could put it into a variable and reuse it here.



core/pom.xml
Lines 19 (patched)
<https://reviews.apache.org/r/69988/#comment298723>

    Please delete dead code



fluent-job/fluent-job-api/pom.xml
Line 80 (original), 80 (patched)
<https://reviews.apache.org/r/69988/#comment298752>

    Should we keep this filename?



fluent-job/pom.xml
Lines 44 (patched)
<https://reviews.apache.org/r/69988/#comment298722>

    Do we really need the version number here? There is no version number in `core/pom.xml`



pom.xml
Lines 1632 (patched)
<https://reviews.apache.org/r/69988/#comment298719>

    Please store the version number in a variable, so we can avoid repeating it multiple times.



pom.xml
Line 1798 (original), 1805 (patched)
<https://reviews.apache.org/r/69988/#comment298718>

    Please store the version number in a variable, so we can avoid repeating it multiple times.



pom.xml
Line 1860 (original), 1866 (patched)
<https://reviews.apache.org/r/69988/#comment298720>

    Please store the version number in a variable, so we can avoid repeating it multiple times.



pom.xml
Line 1865 (original), 1871 (patched)
<https://reviews.apache.org/r/69988/#comment298746>

    Why is it called findbug? Compatibility reasons?



pom.xml
Line 1908 (original), 1914 (patched)
<https://reviews.apache.org/r/69988/#comment298721>

    Please store the version number in a variable, so we can avoid repeating it multiple times.


- Andras Salamon


On Feb. 14, 2019, 4:51 p.m., Kinga Marton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69988/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2019, 4:51 p.m.)
> 
> 
> Review request for oozie and Andras Salamon.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> https://gleclaire.github.io/findbugs-maven-plugin/
> 
> Status: Since Findbugs is no longer maintained, please use Spotbugs which has a Maven plugin.
> 
> The plugin author recommends to migrate to Spotbugs: https://spotbugs.github.io/
> It might worth to investigate this plugin.
> 
> 
> Diffs
> -----
> 
>   bin/test-patch-11-findbugs-diff c884daaa3 
>   client/pom.xml f0f6a1b13 
>   core/pom.xml b6c07d345 
>   fluent-job/fluent-job-api/pom.xml f303b4583 
>   fluent-job/pom.xml bb8861d59 
>   pom.xml 93fffc791 
>   sharelib/git/pom.xml 1c17e48ba 
>   sharelib/oozie/pom.xml a53d335f9 
>   sharelib/spark/pom.xml 76f69034e 
>   tools/pom.xml 72a6c283a 
> 
> 
> Diff: https://reviews.apache.org/r/69988/diff/1/
> 
> 
> Testing
> -------
> 
> Tested manually
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>