ambari-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Doroszlai, Attila (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (AMBARI-19149) Clean up Ambari Server source code warnings
Date Fri, 09 Dec 2016 22:36:58 GMT

    [ https://issues.apache.org/jira/browse/AMBARI-19149?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15736524#comment-15736524
] 

Doroszlai, Attila edited comment on AMBARI-19149 at 12/9/16 10:36 PM:
----------------------------------------------------------------------

[~jonathan.hurley], [~ncole]:

* {{utility}} was introduced for AMBARI-14439 in non-{{org.apache.ambari}} group.  It was
also surprising to me, but didn't want to change that now.
* The two separate {{dependency}} sections for {{com.puppycrawl.tools}} have different {{type}}
and {{scope}}.  We need the {{test-jar}} to be able to use {{BaseCheckTestSupport}} from the
Checkstyle project.  This is not distributed with their regular {{jar}}, since it's needed
only for compiling and running custom checks (eg. {{AvoidTransactionalOnPrivateMethodsCheckTest}}
in our case).
* {{$checkstyle.version}} for the {{checkstyle/test-jar}} dependency, and {{<version>}}
for the regular {{checkstyle}} dependency are inherited from the parent pom, ie. {{ambari-project/pom.xml}}.
* There is no need to always compile the whole root project.  However, when working with a
single module (eg. {{ambari-server}}), one needs to get the dependencies in one of two ways:
** Always make dependencies, too, with the {{-am}} Maven flag.  Note that Ambari Web, Logsearch,
etc. are skipped, since those are not required for the building the server.
** Get the dependencies from some repository.  In our case this means having to install {{utility}},
its parents, and some other Ambari projects in a local repository once.  One needs to re-install
them if the dependencies themselves are updated.
* Now I've noticed that running {{mvn checkstyle:check}} performs the default checks instead
of our custom list, because the configuration was done only for the execution tied to the
test phase, not for the plugin itself.  This is being fixed in AMBARI-19158.

{noformat:title=Always making dependencies}
$ mvn -am -pl ambari-server clean compile
...
[INFO] Reactor Build Order:
[INFO]
[INFO] Ambari Main
[INFO] Apache Ambari Project POM
[INFO] Ambari Views
[INFO] utility
[INFO] ambari-metrics
[INFO] Ambari Metrics Common
[INFO] Ambari Server
{noformat}

{noformat:title=Installing once}
$ mvn -am -pl ambari-server -DskipTests clean install
$ cd ambari-server
$ mvn -DskipTest clean test
$ mvn checkstyle:check
{noformat}


was (Author: adoroszlai):
[~jonathan.hurley], [~ncole]:

* {{utility}} was introduced for AMBARI-14439 in non-{{org.apache.ambari}} group.  It was
also surprising to me, but didn't want to change that now.
* The two separate {{dependency}} sections for {{com.puppycrawl.tools}} have different {{type}}
and {{scope}}.  We need the {{test-jar}} to be able to use {{BaseCheckTestSupport}} from the
Checkstyle project.  This is not distributed with their regular {{jar}}, since it's needed
only for compiling and running custom checks (eg. {{AvoidTransactionalOnPrivateMethodsCheckTest}}
in our case).
* {{$checkstyle.version}} for the {{checkstyle/test-jar}} dependency, and {{<version>}}
for the regular {{checkstyle}} dependency are inherited from the parent pom, ie. {{ambari-project/pom.xml}}.
* There is no need to always compile the whole root project.  However, when working with a
single module (eg. {{ambari-server}}), one needs to get the dependencies in one of two ways:
   * Always make dependencies, too, with the {{-am}} Maven flag.  Note that Ambari Web, Logsearch,
etc. are skipped, since those are not required for the building the server.
   * Get the dependencies from some repository.  In our case this means having to install
{{utility}}, its parents, and some other Ambari projects in a local repository once.  One
needs to re-install them if the dependencies themselves are updated.
* Now I've noticed that running {{mvn checkstyle:check}} performs the default checks instead
of our custom list, because the configuration was done only for the execution tied to the
test phase, not for the plugin itself.  This is being fixed in AMBARI-19158.

{noformat:title=Always making dependencies}
$ mvn -am -pl ambari-server clean compile
...
[INFO] Reactor Build Order:
[INFO]
[INFO] Ambari Main
[INFO] Apache Ambari Project POM
[INFO] Ambari Views
[INFO] utility
[INFO] ambari-metrics
[INFO] Ambari Metrics Common
[INFO] Ambari Server
{noformat}

{noformat:title=Installing once}
$ mvn -am -pl ambari-server -DskipTests clean install
$ cd ambari-server
$ mvn -DskipTest clean test
$ mvn checkstyle:check
{noformat}

> Clean up Ambari Server source code warnings
> -------------------------------------------
>
>                 Key: AMBARI-19149
>                 URL: https://issues.apache.org/jira/browse/AMBARI-19149
>             Project: Ambari
>          Issue Type: Task
>          Components: ambari-server
>            Reporter: Doroszlai, Attila
>            Assignee: Doroszlai, Attila
>             Fix For: 3.0.0
>
>         Attachments: AMBARI-19149-1.patch, IDEA_Ambari_v1.xml
>
>
> Eclipse's default warnings generated for {{ambari-server}} number roughly over 3300.
Out of these, at least half of them are:
> * Unused imports
> * Type safety due to forgotten {{<>}}
> * Missing Serialization IDs from anonymous {{HashMap}} implementations
> * Unused variables
> * {{Capture<T>}} in tests
> * {{switch}} fall-through and missing {{case statements}}: *only makes changes which
won't affect existing functionality*
> This makes spotting actual problems, like missing {{case}} statements a nightmare. We
need to go through and clean out as many of these warnings as possible. 
> Note: With respect to the import cleanup, the IDE of choice should have the following
import order setup. They should always be expanded and never use {{*}}.
> # {{java}}
> # {{javax}}
> # {{org}}
> # {{com}}
> # other



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message