Thanks Roger, appreciate that.
> On Oct 13, 2016, at 3:09 AM, Roger Riggs <roger.ri...@oracle.com> wrote:
>
> Hi,
>
> Looks ok.
>
> Typically, there is a space after the '//' comment characters, it makes it
> easier to read.
> Also, in this case, I don't think the comments help. "Monday" isn't
> important to the test
> and the 'Any Other day' is still 1, not something else.
>
> I would remove those comments.
> Otherwise fine, no need for another webrev.
>
> Thanks, Roger
>
>
> On 10/12/2016 2:16 PM, Anubhav Meena wrote:
>> Hi Roger,
>>
>> Thanks for the suggestion, have made the suggested changes and shifted the
>> tests to /java/time/test/java/time/chrono/TestUmmAlQuraChronology.java.
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.04/
>> <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.04/>
>>
>> Thanks,
>> Anubhav
>>
>>> On Oct 12, 2016, at 8:45 PM, Roger Riggs <roger.ri...@oracle.com
>>> <mailto:roger.ri...@oracle.com>> wrote:
>>>
>>> Hi Anubhav,
>>>
>>> In general, I agree with Stephen that the tests should be testing an
>>> algorithm against facts.
>>> Embedding an algorithm in the test increases the risk that the test will
>>> just replicate the
>>> implementation code and therefor not be much of a test.
>>>
>>> Though in this case, the specification of aligned day of week is of a
>>> computation.
>>> If the test were to independently compute the correct answer, it would be
>>> valid as a 'tck' test.
>>>
>>> Since the Hijrah calendar is data driven, the tests should be in
>>> /java/time/test/java/time/chrono/TestUmmAlQuraChronology.java.
>>> Tests in java/time/tck/... should correspond directly to specified
>>> behaviors.
>>> In this case, the algorithm is specified but the test is data dependent.
>>> (Perhaps a gray area).
>>>
>>> Thanks, Roger
>>>
>>>
>>> On 10/12/2016 11:03 AM, Anubhav Meena wrote:
>>>> Hi Stephen,
>>>>
>>>> Have incorporated the changes you suggested. Updated webrev is available
>>>> here
>>>> http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.03/
>>>> <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.03/>
>>>>
>>>> Please review and suggest if anymore changes are required.
>>>>
>>>> Thanks,
>>>> Anubhav
>>>>
>>>>> On Oct 12, 2016, at 3:21 PM, Anubhav Meena <anubhav.me...@oracle.com
>>>>> <mailto:anubhav.me...@oracle.com>> wrote:
>>>>>
>>>>> Gentle reminder.
>>>>>
>>>>>> On Oct 7, 2016, at 2:12 PM, Anubhav Meena <anubhav.me...@oracle.com
>>>>>> <mailto:anubhav.me...@oracle.com>> wrote:
>>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> Please review.
>>>>>> Bug Id : https://bugs.openjdk.java.net/browse/JDK-8163330
>>>>>> <https://bugs.openjdk.java.net/browse/JDK-8163330>
>>>>>>
>>>>>> Issue: The HijrahDate class incorrectly calculates the
>>>>>> aligned-day-of-week field. It based the calculation on the day-of-week,
>>>>>> when it should be based on the day-of-month.
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/
>>>>>> <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.02/>
>>>>>> --
>>>>>> Thanks and Regards,
>>>>>> Anubhav
>>>>>
>>>>
>>>
>>
>
