Maybe it would help to rename TestOrdering to TestComparators ...

-- Jon

On 6/9/20 1:33 AM, Hannes Wallnoefer wrote:
Hi Kumar,

Thanks a lot for the hint! It is sometimes hard to figure out where to put 
tests, and TestOrdering is without doubt the right place here. I’ve already 
pushed this, but I’ll file another issue to move the test with added checks for 
ordering.

Hannes

Am 08.06.2020 um 19:56 schrieb Kumar Srinivasan <kusriniva...@vmware.com>:

Hi Hannes,

Apologies for my tardiness. I think this might require a test case in 
TestOrdering.
TestOrdering was created to ensure the comparator produces consistent ordering.

Kumar


On Jun 6, 2020, at 7:11 AM, Jonathan Gibbons <jonathan.gibb...@oracle.com> 
wrote:

On 6/5/20 9:10 AM, Hannes Wallnoefer wrote:
Thanks!

Am 04.06.2020 um 16:51 schrieb Jonathan Gibbons <jonathan.gibb...@oracle.com>:

+1

I'll comment that the edits are inconsistent with using `{ }` around 
single-statements after `if (...)`. Note sure if that was deliberate or not.
I know. I usually try to be consistent with the style of the method I’m adding 
to, which in this case unfortunately varies within a single class.

Hannes
Consistency is good ;-)

-- Jon



Test: yay for Text Blocks.

-- Jon

On 6/4/20 7:19 AM, Hannes Wallnoefer wrote:
Please review:

JBS: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8246429&amp;data=02%7C01%7Ckusrinivasan%40vmware.com%7C0e72eb7ac9ea4fbcb2a208d80a23e56f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637270496577278712&amp;sdata=gXPqYbmKk%2F02PtWFEmC9SekXezCCN3H8uOcbV3Xt0dc%3D&amp;reserved=0
Webrev: 
https://nam04.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~hannesw%2F8246429%2Fwebrev.00%2F&amp;data=02%7C01%7Ckusrinivasan%40vmware.com%7C0e72eb7ac9ea4fbcb2a208d80a23e56f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637270496577278712&amp;sdata=Kq70LDFQ7Rpv%2FlTrYX9ele3PRLKIYKUNNPkyEeIM62g%3D&amp;reserved=0

I originally fixed this as part of 8198705: Javadoc search needs a fix to 
handle duplicate package names in different modules, but it is a distinct issue 
so I filed a separate bug for it.

The fix adds an additional final step to element comparisons, which is to 
compare the name of the modules containing the elements. So far the final step 
in most comparators was to compare the fully qualified name of the elements.

Hannes

Reply via email to