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

Zoltan Siegl commented on YARN-8644:
------------------------------------

Hi [~snemeth]!
 Thanks, I am done with the review. I have a few minor comments.

 * 
{{org/apache/hadoop/yarn/server/resourcemanager/rmapp/AppCreationTestHelper.java:295}}
{code:java}
Assert.assertEquals("application tracking url is not correct",
        null, application.getTrackingUrl());
{code}
Shouldn't we consider using assertNull here?

 * 
{{org/apache/hadoop/yarn/server/resourcemanager/rmapp/TestRMAppTransitions.java:951}}
{code:java}
Assert.assertTrue("RMAppImpl: can't handle " + rmAppEventType
                                 + " at state " + state, false);
{code}
If you are good boy-scouting around here anyways, might this be considered to 
become a {{Assert.fail}}?

Everything else looks good to me, even these are just minor things that you 
could consider to touch if you create a new patch anyways.
 

> Make RMAppImpl$FinalTransition more readable + add more test coverage
> ---------------------------------------------------------------------
>
>                 Key: YARN-8644
>                 URL: https://issues.apache.org/jira/browse/YARN-8644
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Szilard Nemeth
>            Assignee: Szilard Nemeth
>            Priority: Minor
>         Attachments: YARN-8644.001.patch, YARN-8644.002.patch
>
>




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org

Reply via email to