> On Dec. 15, 2016, 10:56 a.m., Nate Cole wrote:
> > ambari-server/checkstyle.xml, lines 39-41
> > <https://reviews.apache.org/r/54735/diff/4/?file=1585584#file1585584line39>
> >
> > This check seems a bit restrictive, both in having to name the variable
> > "ignored" and requiring some justification text. The code can speak for
> > itself.
It requires either, not both. Examples from the patch:
```
} catch (AmbariException e) {
+ // null clusterId reported and handled by the listener
(DistributeRepositoriesActionListener)
}
```
and
```
try {
reason = Integer.parseInt(statusCode);
- } catch (Exception e) {
+ } catch (Exception ignored) {
}
```
- Attila
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54735/#review159273
-----------------------------------------------------------
On Dec. 15, 2016, 7:44 a.m., Attila Doroszlai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54735/
> -----------------------------------------------------------
>
> (Updated Dec. 15, 2016, 7:44 a.m.)
>
>
> Review request for Ambari, Jonathan Hurley, Laszlo Puskas, Miklos Gergely,
> and Nate Cole.
>
>
> Bugs: AMBARI-19149
> https://issues.apache.org/jira/browse/AMBARI-19149
>
>
> Repository: ambari
>
>
> Description
> -------
>
> * Empty `synchronized` blocks in `ServiceInfo` seems to be bug
> * Introduced `Closeables.closeSilently` to get rid of duplicated try-catch
> logic with empty catch
> * Exceptions caught and ignored should be named `ignored`, ideally catch
> block should state why it's OK to ignore
> * Added check for empty blocks and unnecessary nested blocks
>
>
> Diffs
> -----
>
> ambari-server/checkstyle.xml 0c66556d513d7e3da0f969be584d666ba00b9c6c
>
> ambari-server/src/main/java/org/apache/ambari/server/agent/HeartbeatProcessor.java
> ed84e459a5c8e84a660e4b08e1f9cbe01624f582
>
> ambari-server/src/main/java/org/apache/ambari/server/api/services/serializers/CsvSerializer.java
> 92d8b69af5536b83c975cb0cb05b936aad866e19
>
> ambari-server/src/main/java/org/apache/ambari/server/bootstrap/BSHostStatusCollector.java
> 661f2845080c6202194e480ab84b6b79ca11fafa
>
> ambari-server/src/main/java/org/apache/ambari/server/bootstrap/BSRunner.java
> 6c77dee3bdff9d442e9efabf7321daadb8b076ee
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/gsinstaller/ClusterDefinition.java
> 6f9876abfa90049997fac5d7cce6a8abe6742f63
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractProviderModule.java
> 5d462c56fbfc0cfd7b098c79d7553a3fe6841dc1
>
> ambari-server/src/main/java/org/apache/ambari/server/metadata/CachedRoleCommandOrderProvider.java
> e2b44cab6f874b1f00d270db2344aaf89acd09e7
>
> ambari-server/src/main/java/org/apache/ambari/server/metadata/RoleCommandOrder.java
> 58675aea7b5454ce4be9f0bdcf183fe1b2788367
>
> ambari-server/src/main/java/org/apache/ambari/server/security/encryption/CredentialProvider.java
> 764380409c96d640e6b79bfedc8ba66d21724ae0
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/IPAKerberosOperationHandler.java
> 8cdef0e01dfd2495f8d73cbea19dc9a0f729cff4
>
> ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandler.java
> 2880f6e0c3099e0a86f58da8cae9e1682dd484e3
>
> ambari-server/src/main/java/org/apache/ambari/server/stack/QuickLinksConfigurationModule.java
> 9554f0c36b0338410482b4d157975c3971af220f
> ambari-server/src/main/java/org/apache/ambari/server/stack/ThemeModule.java
> f07efea4052e5fbbc2dca45df61ca8d6b1c4a356
>
> ambari-server/src/main/java/org/apache/ambari/server/state/ConfigHelper.java
> 3fb036da92b23b4a6caf7dafbc5c36c9e320511e
> ambari-server/src/main/java/org/apache/ambari/server/state/ServiceInfo.java
> 00c7a64217bcf13ecb40ae848310ef85b1fcef90
>
> ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java
> b62c834cb4125ee33d54860ad72c263bd49a4f85
>
> ambari-server/src/main/java/org/apache/ambari/server/upgrade/SchemaUpgradeHelper.java
> 99e50fb630c34ea2b00beb0d7f693835908a7f6d
> ambari-server/src/main/java/org/apache/ambari/server/utils/Closeables.java
> PRE-CREATION
>
> ambari-server/src/main/java/org/apache/ambari/server/view/ViewDirectoryWatcher.java
> 171e873cae79dda2b01346a969eb426f1a783946
> ambari-server/src/main/java/org/apache/ambari/server/view/ViewRegistry.java
> 551096ca466ede01d8eaa12a19dde4738857171c
>
> ambari-server/src/main/java/org/apache/ambari/server/view/ViewURLStreamProvider.java
> 24d3527f411286ff138482b84ad4715a08508c5b
>
> ambari-server/src/test/java/org/apache/ambari/server/utils/CloseablesTest.java
> PRE-CREATION
>
> Diff: https://reviews.apache.org/r/54735/diff/
>
>
> Testing
> -------
>
> ```
> $ mvn -am -pl ambari-server -Del.log=OFF -DskipPythonTests clean test
> ...
> Tests run: 4797, Failures: 0, Errors: 0, Skipped: 37
> ...
> [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0
> approved: 5367 licence.
> ...
> [INFO] --- maven-checkstyle-plugin:2.17:check (checkstyle) @ ambari-server ---
> [INFO] Starting audit...
> Audit done.
> [INFO]
> ------------------------------------------------------------------------
> [INFO] Reactor Summary:
> [INFO]
> [INFO] Ambari Main ....................................... SUCCESS [6.464s]
> [INFO] Apache Ambari Project POM ......................... SUCCESS [0.036s]
> [INFO] Ambari Views ...................................... SUCCESS [2.253s]
> [INFO] utility ........................................... SUCCESS [1.178s]
> [INFO] ambari-metrics .................................... SUCCESS [2.299s]
> [INFO] Ambari Metrics Common ............................. SUCCESS [4.382s]
> [INFO] Ambari Server ..................................... SUCCESS
> [23:27.066s]
> [INFO]
> ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> ```
>
>
> Thanks,
>
> Attila Doroszlai
>
>
