ambari-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tom Beerbower" <tbeerbo...@hortonworks.com>
Subject Re: Review Request 27590: Upgrade Execute: Create API endpoint for upgrades and upgrade items
Date Wed, 05 Nov 2014 02:03:23 GMT


> On Nov. 4, 2014, 11:22 p.m., Jonathan Hurley wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java,
line 106
> > <https://reviews.apache.org/r/27590/diff/1/?file=749671#file749671line106>
> >
> >     Hungarian anyone?
> 
> Alejandro Fernandez wrote:
>     I prefer camelCase for Java, but ok with other notations as long as they are used
consistently.
> 
> Jonathan Hurley wrote:
>     I've been coding with camelCase + hungarian for over a decade; it's hard to quit
cold turkey from the hungarian. It would avoid cases like this where you'd have sUpgradeId
(string) and iUpgradeId (int) and upgradeId (Object). But I think that there's just no way
to change now as the codebase is too large.

I'll admit that in this case it might make the naming easier but I generally dislike hungarian
because in some ways it makes the code less readable and harder to maintain. 

While we are on the subject of readability... I don't understand the whole 'null == <variable>'
convention.  It had a purpose in C programming.  It serves no purpose in Java that I know
of except to make the code less readable.  If I read the above code aloud in English, I would
say "if upgrade id equals null", not the other way around.

Sorry for the rant.


- Tom


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


On Nov. 4, 2014, 9:55 p.m., Nate Cole wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27590/
> -----------------------------------------------------------
> 
> (Updated Nov. 4, 2014, 9:55 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Tom Beerbower.
> 
> 
> Bugs: AMBARI-8144
>     https://issues.apache.org/jira/browse/AMBARI-8144
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Added stubs for /api/v1/clusters/c1/upgrades
> 
> This test does not provide full DB connectivity.  It's only meant to unblock other teams
from having an endpoint to work with.  There will be many many more changes over the course
of the Upgrades initiative
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/StaticallyInject.java PRE-CREATION

>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceInstanceFactoryImpl.java
f5de481 
>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeItemResourceDefinition.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/api/resources/UpgradeResourceDefinition.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java
ab71ce2 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeItemService.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/UpgradeService.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariServer.java 14e5dc8

>   ambari-server/src/main/java/org/apache/ambari/server/controller/ControllerModule.java
0974f6f 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/DefaultProviderModule.java
de63db5 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeItemResourceProvider.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/UpgradeResourceProvider.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/spi/Resource.java b665813

>   ambari-server/src/main/java/org/apache/ambari/server/orm/dao/UpgradeDAO.java PRE-CREATION

>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeEntity.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/orm/entities/UpgradeItemEntity.java
PRE-CREATION 
>   ambari-server/src/main/java/org/apache/ambari/server/state/UpgradeState.java PRE-CREATION

>   ambari-server/src/test/java/org/apache/ambari/server/orm/dao/UpgradeDAOTest.java PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/27590/diff/
> 
> 
> Testing
> -------
> 
> Only basic test added, more to come as functionality fills out.
> 
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 30:48.302s
> [INFO] Finished at: Tue Nov 04 16:54:11 EST 2014
> [INFO] Final Memory: 29M/248M
> [INFO] ------------------------------------------------------------------------
> 
> 
> Thanks,
> 
> Nate Cole
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message