RFR: 8189843: Missing "id" attributes in table rows

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8189843: Missing "id" attributes in table rows

Jonathan Gibbons
Please review a simple fix to fix the fix for JDK-8182257.

The original problem was the appearance of duplicate uses of an "id". The original fix was flawed because it got rid of both instances of each duplicated id. This fix better disables the use of ids in the property summary table, while retaining them in the method table.

The primary purpose of this changeset is to introduce the new tests. The source code will soon be further improved with the introduction of a new Table builder class.

In this changeset, the fix is to pass down a parameter indicating the "kind" of the summary table, such that ids are suppressed in property summary tables.

JBS: https://bugs.openjdk.java.net/browse/JDK-8182257
Webrev: http://cr.openjdk.java.net/~jjg/8189843/webrev.00/

-- Jon

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8189843: Missing "id" attributes in table rows

Kumar Srinivasan

Hi Jon,

Your changes for the fix in itself if ok.

However, I was looking at this:
public boolean isProperty(String name) {
    return configuration.javafx && name.endsWith("Property");
}
I wonder if we should have a more robust check.

Kumar
 
Please review a simple fix to fix the fix for JDK-8182257.

The original problem was the appearance of duplicate uses of an "id". The original fix was flawed because it got rid of both instances of each duplicated id. This fix better disables the use of ids in the property summary table, while retaining them in the method table.

The primary purpose of this changeset is to introduce the new tests. The source code will soon be further improved with the introduction of a new Table builder class.

In this changeset, the fix is to pass down a parameter indicating the "kind" of the summary table, such that ids are suppressed in property summary tables.

JBS: https://bugs.openjdk.java.net/browse/JDK-8182257
Webrev: http://cr.openjdk.java.net/~jjg/8189843/webrev.00/

-- Jon


Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8189843: Missing "id" attributes in table rows

Jonathan Gibbons

Yes, but that could/should be done separately.

I'd prefer to get VMM2 in place first.

-- Jon


On 10/25/17 6:57 AM, Kumar Srinivasan wrote:

Hi Jon,

Your changes for the fix in itself if ok.

However, I was looking at this:
public boolean isProperty(String name) {
    return configuration.javafx && name.endsWith("Property");
}
I wonder if we should have a more robust check.

Kumar
 
Please review a simple fix to fix the fix for JDK-8182257.

The original problem was the appearance of duplicate uses of an "id". The original fix was flawed because it got rid of both instances of each duplicated id. This fix better disables the use of ids in the property summary table, while retaining them in the method table.

The primary purpose of this changeset is to introduce the new tests. The source code will soon be further improved with the introduction of a new Table builder class.

In this changeset, the fix is to pass down a parameter indicating the "kind" of the summary table, such that ids are suppressed in property summary tables.

JBS: https://bugs.openjdk.java.net/browse/JDK-8182257
Webrev: http://cr.openjdk.java.net/~jjg/8189843/webrev.00/

-- Jon



Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8189843: Missing "id" attributes in table rows

Bhavesh Patel
In reply to this post by Jonathan Gibbons


Looks good.


Regards,

Bhavesh.


On 10/24/2017 3:34 PM, Jonathan Gibbons wrote:
Please review a simple fix to fix the fix for JDK-8182257.

The original problem was the appearance of duplicate uses of an "id". The original fix was flawed because it got rid of both instances of each duplicated id. This fix better disables the use of ids in the property summary table, while retaining them in the method table.

The primary purpose of this changeset is to introduce the new tests. The source code will soon be further improved with the introduction of a new Table builder class.

In this changeset, the fix is to pass down a parameter indicating the "kind" of the summary table, such that ids are suppressed in property summary tables.

JBS: https://bugs.openjdk.java.net/browse/JDK-8182257
Webrev: http://cr.openjdk.java.net/~jjg/8189843/webrev.00/

-- Jon