RFR: CODETOOLS-7901044: Add next/prev links to navigate between diffs

[Daniel Fuchs]

Hi Bengt,

This is a really cool feature!

I have downloaded your patch and applied it in my
local webrev repo - and played with the new
webrev in a couple of repositories where I have
pending uncommitted changes.

This seems to work well.

I have used:

   webrev -fN from top level repo
   webrev -N  from jdk repo
   webrev -fN from jdk repo (with changes in test/closed)

I also had a cursory look at your patch. The only thing
I noticed is that 'typeset' is not consistently used in
the original webrev file. I'm not sure whether that's
intended or not.
I believe that's ok - but I've been bitten by not using
typeset before ;-( - maybe - for the sake of uniformity -
you could use typeset at least in those functions that
already use  it for their parameters? For instance
lines 532-534, and possibly line 560 as well?
Or did you intentionally not used it?

Also you should increment the version of webrev.
IIRC that would be 25.6:

- WEBREV_UPDATED=25.5-hg+openjdk.java.net
+ WEBREV_UPDATED=25.6-hg+openjdk.java.net

best regards,

-- daniel

On 01/10/14 10:56, Bengt Rutisson wrote:
>
> Hi everyone,
>
> (This is a patch for webrev, but I am cross posting to the JDK 9 dev
> list since I'm not sure how many are on the webrev-dev list and most JDK
> developers might have opinions about this change.)
>
> Can I get a couple of reviews for this small enhancement to the webrev
> tool?
>
> CODETOOLS-7901044: Add next/prev links to navigate between diffs
> https://bugs.openjdk.java.net/browse/CODETOOLS-7901044
>
> Webrev:
> http://cr.openjdk.java.net/~brutisso/CODETOOLS-7901044/webrev.00/
>
>
> Background:
> When browsing review requests that span many files it is quite time
> consuming to have to go back to the index page to view to the next or
> previous file. This patch adds links to the next and previous files in
> the diff views (including frames).
>
> The above webrev is generated with this new version, but since it only
> contains a single file it is not that interesting. Here is an example of
> what the new webrev looks like for a larger change (the G1 class
> unloading change):
>
> http://cr.openjdk.java.net/~brutisso/CODETOOLS-7901044/webrev.00/example/
>
> Thanks,
> Bengt
>