[
https://issues.apache.org/jira/browse/YARN-955?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13820652#comment-13820652
]
Mayank Bansal commented on YARN-955:
------------------------------------
Thanks [~zjshen] fir review
bq. 1. It's not necessary, and shouldn't be public.
Done
bq. 2. It's not a recommended way to construct pb instances. Please use
XXXX.newInstance() for all the responses here.
Done
bq. 3. Why the reference of the implementation is used? Should we use
ApplicationHistoryManager, the interface, instead?
Done
bq. 4. In TestApplicationHistoryClientService, please write multiple instances,
and test getXXXXs methods as well.
Done
bq. 5. Add @VisibleForTesting
Done
bq. 6. Unwrap the method and put it directly in ApplicationHistoryServer.main
Writing everything i main function is not a good prectise however keeping away
from main and have a seprate method I think is better
Thanks,
Mayank
> [YARN-321] Implementation of ApplicationHistoryProtocol
> -------------------------------------------------------
>
> Key: YARN-955
> URL: https://issues.apache.org/jira/browse/YARN-955
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Vinod Kumar Vavilapalli
> Assignee: Mayank Bansal
> Attachments: YARN-955-1.patch, YARN-955-2.patch, YARN-955-3.patch,
> YARN-955-4.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.1#6144)
