Junping Du commented on YARN-2103:

Thanks [~decster] for the patch and [~ozawa] for review.
bq. How about adding concrete tests as a first step of generic tests on 
YARN-2051? What do you think, Junping Du?
That sounds good to me. We should add some concrete tests for some highly 
suspected PBImpls as the first step and refactor to/add more generic tests 
later. In this case, the original constructor (the one without parameter) is 
wrong as the object created with viaProto=false and builder to be null at the 
same time. We should add a unit test here. 

On code format, please try to follow our code convention in hadoop wiki - 
Sun/Oracle java programming convention (only difference is using 2 indentations 
instead of 4). In this convention, we have:
Note: if statements always use braces {}. Avoid the following error-prone form:
if (condition) //AVOID! THIS OMITS THE BRACES {}!
So may be we should try to be right for all new added code instead of staying 
the same with legacy code?

> Inconsistency between viaProto flag and initial value of 
> SerializedExceptionProto.Builder
> -----------------------------------------------------------------------------------------
>                 Key: YARN-2103
>                 URL: https://issues.apache.org/jira/browse/YARN-2103
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: Binglin Chang
>            Assignee: Binglin Chang
>         Attachments: YARN-2103.v1.patch
> Bug 1:
> {code}
>   SerializedExceptionProto proto = SerializedExceptionProto
>       .getDefaultInstance();
>   SerializedExceptionProto.Builder builder = null;
>   boolean viaProto = false;
> {code}
> Since viaProto is false, we should initiate build rather than proto
> Bug 2:
> the class does not provide hashcode() and equals() like other PBImpl records, 
> this class is used in other records, it may affect other records' behavior. 

This message was sent by Atlassian JIRA

Reply via email to