Re: RFR: 8179370: Replace use of , and tags in java.base

2017-04-26 Thread joe darcy
Looks better; thanks Jon, -Joe On 4/26/2017 6:49 PM, Jonathan Gibbons wrote: Updated webrev to address Joe's suggestion to try harder to use {@code} as a substitute for . http://cr.openjdk.java.net/~jjg/8179370/webrev.01 The modified sed script has a new first line: s|\([^&<>]*\)|{@code

Re: RFR: 8179370: Replace use of , and tags in java.base

2017-04-26 Thread Jonathan Gibbons
Updated webrev to address Joe's suggestion to try harder to use {@code} as a substitute for . http://cr.openjdk.java.net/~jjg/8179370/webrev.01 The modified sed script has a new first line: s|\([^&<>]*\)|{@code \1}|g s|||g s|||g s|<\\/tt>|<\\/code>|g s|\(]*\)>\([^<]*\)|\1

Re: RFR: 8179370: Replace use of , and tags in java.base

2017-04-26 Thread Mandy Chung
Looks okay. Mandy > On Apr 26, 2017, at 5:50 PM, Jonathan Gibbons > wrote: > > Please review these mostly simple changes to replace HTML tags which are not > valid in HTML 5 in public doc comments in java.base. > > As with the previous changes, the changes were

Re: RFR: 8179370: Replace use of , and tags in java.base

2017-04-26 Thread Jonathan Gibbons
Joe, Yes, there are occurrences here that require instead of {@code}, because of the presence of HTML entities. It's about 50/50. I'd prefer to stay with mechanical updates as much as possible for these bulk updates, as compared to manual updates, but I may be able to improve the sed

Re: RFR: 8179370: Replace use of , and tags in java.base

2017-04-26 Thread Joseph D. Darcy
Hi Jon, I'd prefer if the "foo" were replaced with "{@code tt}" rather than "foo"- none of the tricky cases which preclude use of {@code } use seem to be present here - but will approve the changeset in its current form too. Cheers, -Joe On 4/26/2017 5:50 PM, Jonathan Gibbons wrote:

RFR: 8179370: Replace use of , and tags in java.base

2017-04-26 Thread Jonathan Gibbons
Please review these mostly simple changes to replace HTML tags which are not valid in HTML 5 in public doc comments in java.base. As with the previous changes, the changes were done mechanically, using the following sed script: s|||g s|||g s|<\\/tt>|<\\/code>|g s|\(]*\)>\([^<]*\)|\1

[JDK9] RFR 8167229: Improve VarHandle documentation

2017-04-26 Thread Paul Sandoz
Hi, Please review some documentation changes to VarHandle: http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8167229-varhandle-docs/webrev/index.html There are no changes to the specification. A significant portion of the changes are based on feedback from Alex who observed it’s possible to

Re: RFR: 8179367: update use of align, valign attributes in java.base to use style attribute

2017-04-26 Thread Martin Buchholz
On Wed, Apr 26, 2017 at 3:52 PM, Jonathan Gibbons < jonathan.gibb...@oracle.com> wrote: > Martin, > > I would agree that in a stylesheet .css file it would be better to include > the terminating semicolon, especially when the items are laid out on > multiple lines. > > Here, they're in inline

Re: RFR: 8179367: update use of align, valign attributes in java.base to use style attribute

2017-04-26 Thread Mandy Chung
The patch looks okay to me. Mandy > On Apr 26, 2017, at 3:31 PM, Jonathan Gibbons > wrote: > > Please review the following conceptually simple fix to change use of the > align and valign attributes in table-related tags to inline style attributes > in the public

Re: RFR: 8179367: update use of align, valign attributes in java.base to use style attribute

2017-04-26 Thread Jonathan Gibbons
Martin, I would agree that in a stylesheet .css file it would be better to include the terminating semicolon, especially when the items are laid out on multiple lines. Here, they're in inline style attributes, so I don't think the trailing semicolon is so important. A better cleanup, in

Re: RFR: 8179367: update use of align, valign attributes in java.base to use style attribute

2017-04-26 Thread Martin Buchholz
LGTM On Wed, Apr 26, 2017 at 3:47 PM, Jonathan Gibbons < jonathan.gibb...@oracle.com> wrote: > Martin, > > ';' can be used either way ... it's an optional terminator on the last > item. w3schools typically shows without the ';' when it is not necessary. > > -- Jon > > > On 04/26/2017 03:40 PM,

Re: RFR: 8179367: update use of align, valign attributes in java.base to use style attribute

2017-04-26 Thread Jonathan Gibbons
Martin, ';' can be used either way ... it's an optional terminator on the last item. w3schools typically shows without the ';' when it is not necessary. -- Jon On 04/26/2017 03:40 PM, Martin Buchholz wrote: This looks good. jsr166 maintainers will merge into jsr166 CVS. I'm unsure

Re: RFR: 8179367: update use of align, valign attributes in java.base to use style attribute

2017-04-26 Thread Martin Buchholz
http://stackoverflow.com/questions/11939595/leaving-out-the-last-semicolon-of-a-css-block Terminating with a semicolon consistently seems to be good practice even if it's not necessary. On Wed, Apr 26, 2017 at 3:40 PM, Martin Buchholz wrote: > This looks good. > > jsr166

Re: RFR: 8179367: update use of align, valign attributes in java.base to use style attribute

2017-04-26 Thread Martin Buchholz
This looks good. jsr166 maintainers will merge into jsr166 CVS. I'm unsure whether the ";" should be used as separator or terminator. I would have terminated every style with a ";" On Wed, Apr 26, 2017 at 3:31 PM, Jonathan Gibbons < jonathan.gibb...@oracle.com> wrote: > Please review the

RFR: 8179367: update use of align, valign attributes in java.base to use style attribute

2017-04-26 Thread Jonathan Gibbons
Please review the following conceptually simple fix to change use of the align and valign attributes in table-related tags to inline style attributes in the public doc comments for the contents of java.base module. This is for compatibility with HTML 5. The change was done mechanically with

Re: JDK 9 test-only RFR of 8179247: java/util/zip/TestExtraTime.java: add some instrumentation which might illuminate the failure of 2016-09-14

2017-04-26 Thread Roger Riggs
Hi Brian, Looks good Thanks, Roger On 4/26/2017 4:57 PM, Brian Burkhalter wrote: Hi Roger, Here is an updated patch containing the suggested changes: http://cr.openjdk.java.net/~bpb/8179247/webrev.01/ Thanks, Brian On Apr 26,

Re: JDK 9 test-only RFR of 8179247: java/util/zip/TestExtraTime.java: add some instrumentation which might illuminate the failure of 2016-09-14

2017-04-26 Thread Brian Burkhalter
Hi Roger, Here is an updated patch containing the suggested changes: http://cr.openjdk.java.net/~bpb/8179247/webrev.01/ Thanks, Brian On Apr 26, 2017, at 12:00 PM, Brian Burkhalter wrote: >> On the original webrev: >> In addition to checking if the directory

Re: RFR: 8179364: update "

2017-04-26 Thread joe darcy
Looks fine Jon; thanks, -Joe On 4/26/2017 1:30 PM, Jonathan Gibbons wrote: Please review the following conceptually simple fix to change where $files was derived from the set of files reported by "javadoc -html5" as containing the use of the name attribute. The covers files in the

RFR: 8179364: update "

2017-04-26 Thread Jonathan Gibbons
Please review the following conceptually simple fix to change where $files was derived from the set of files reported by "javadoc -html5" as containing the use of the name attribute. The covers files in the following packages: java.base java/io java.base java/lang java.base

Re: JDK 9 test-only RFR of 8179247: java/util/zip/TestExtraTime.java: add some instrumentation which might illuminate the failure of 2016-09-14

2017-04-26 Thread Brian Burkhalter
On Apr 26, 2017, at 11:14 AM, Roger Riggs wrote: > The test.dir property seems like a convenience for the test developer. > It is not defined by jtreg in the tag specification as are the other > properties [1]. > It is not widely used in tests but could be

Re: JDK 9 test-only RFR of 8179247: java/util/zip/TestExtraTime.java: add some instrumentation which might illuminate the failure of 2016-09-14

2017-04-26 Thread Roger Riggs
Hi, The test.dir property seems like a convenience for the test developer. It is not defined by jtreg in the tag specification as are the other properties [1]. It is not widely used in tests but could be understandable when manually testing to avoid cluttering the working directory with test

Re: JDK 9 RFR: JDK-8177146 MethodHandles.Lookup::bind allows illegal protected access

2017-04-26 Thread Claes Redestad
Hi, looks good to me! Nit: I prefer descriptive test names, but can't find the creativity to suggest a better name in this case. Thanks! /Claes Ron Pressler skrev: (26 april 2017 18:50:16 CEST) >Hi. >Please review, > >Bug:

Re: JDK 9 RFR: JDK-8177146 MethodHandles.Lookup::bind allows illegal protected access

2017-04-26 Thread Vladimir Ivanov
Ron, The fix looks good. One request: please, try to avoid bug ids in test names. It's much easier to work with tests when they have meaningful names and there's already @bug jtreg tag to refer to the fixed bug from the test. Best regards, Vladimir Ivanov On 4/26/17 7:50 PM, Ron Pressler

Re: JDK 9 RFR: JDK-8177146 MethodHandles.Lookup::bind allows illegal protected access

2017-04-26 Thread Roger Riggs
Hi, Not to be really picky but the MethodHandles source file really stresses the coding conventions for long lines, recommended line breaks, etc. But not a comment on the proposed changes. Roger On 4/26/2017 1:00 PM, Paul Sandoz wrote: On 26 Apr 2017, at 09:50, Ron Pressler

Re: JDK 9 RFR: JDK-8177146 MethodHandles.Lookup::bind allows illegal protected access

2017-04-26 Thread Paul Sandoz
> On 26 Apr 2017, at 09:50, Ron Pressler wrote: > > Hi. > Please review, > > Bug: https://bugs.openjdk.java.net/browse/JDK-8177146 > Webrev: > http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8177146-bind-protected-method/webrev/ > +1 Paul. > - > > The patch

JDK 9 RFR: JDK-8177146 MethodHandles.Lookup::bind allows illegal protected access

2017-04-26 Thread Ron Pressler
Hi. Please review, Bug: https://bugs.openjdk.java.net/browse/JDK-8177146 Webrev: http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8177146-bind-protected-method/webrev/ - The patch contains a few changes: 1. The method `getDirectMethodCommon`, was changed so that if narrowing due to

Re: JDK 9 test-only RFR of 8179247: java/util/zip/TestExtraTime.java: add some instrumentation which might illuminate the failure of 2016-09-14

2017-04-26 Thread Brian Burkhalter
I was wondering whether test.dir is there to allow to direct to large disks if large files are involved. This is a general observation, not just for this test. Brian On Apr 26, 2017, at 6:37 AM, Roger Riggs wrote: > I would keep it simple and use the current form. It

Re: JDK 9 test-only RFR of 8179247: java/util/zip/TestExtraTime.java: add some instrumentation which might illuminate the failure of 2016-09-14

2017-04-26 Thread Roger Riggs
Hi, I would keep it simple and use the current form. It make it easy to run the tests standalone. dir = System.getProperty("test.dir", ".") Adding in the additional "user.dir" variant just complicates the code and understanding without improving the use cases. (by default user.dir is

Re: JDK 9 test-only RFR of 8179247: java/util/zip/TestExtraTime.java: add some instrumentation which might illuminate the failure of 2016-09-14

2017-04-26 Thread Daniel Fuchs
Hi, Looking at the test - is it possible that "test.dir" is used here simply to make it easier to run the test outside of jtreg - e.g. from within an IDE like e.g. NetBeans? Then if so "test.dir" has probably nothing to do with jtreg... If you run a test (I mean its main() method) from within