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