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

Peter Bacsko commented on YARN-9133:
------------------------------------

Thanks for the patch [~snemeth]. I have some comments:

* Is it really a good idea to have {{instance}} as "public"? I'd rather see it 
private with a setter, which is package-private.
* If {{GpuDiscoverer}} is singleton, having a private constructor would make 
sense
* {{Assert.assertTrue()}} and {{assertEquals()}} are mixed up a little bit. 
Let's static import {{assertTrue()}} as well.

> Make tests more easy to comprehend in TestGpuResourceHandler
> ------------------------------------------------------------
>
>                 Key: YARN-9133
>                 URL: https://issues.apache.org/jira/browse/YARN-9133
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Szilard Nemeth
>            Assignee: Szilard Nemeth
>            Priority: Major
>         Attachments: YARN-9133.001.patch, YARN-9133.001.patch
>
>
> Tests are not quite easy to read: 
> - Some more helper methods would improve readability.
> - Eliminating the boolean flag that controls if docker is used would also 
> improve readability and clarity.



--
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