RFR: CODETOOLS-7902478 - Add webrev flag to avoid printing workspace (Was: Ability to not display local filesystem path in the generated webrev?)
jesper.wilhelmsson at oracle.com
jesper.wilhelmsson at oracle.com
Thu Jun 13 07:50:25 UTC 2019
Thank you Jon!
/Jesper
> On 13 Jun 2019, at 09:10, Jonathan Gibbons <jonathan.gibbons at oracle.com> wrote:
>
> Jesper,
>
> I'll look at sponsoring this change into the repo tomorrow, Thursday.
>
> -- Jon
>
> On 6/12/19 8:57 PM, Jaikiran Pai wrote:
>> I'm not a reviewer, but this change looks good to me. I applied this
>> patch locally and ran the tool with both -w and without it. Works as
>> expected.
>>
>> Thank you Jesper for implementing this.
>>
>> -Jaikiran
>>
>> On 13/06/19 3:48 AM, jesper.wilhelmsson at oracle.com wrote:
>>>> On 12 Jun 2019, at 15:02, Jaikiran Pai <jai.forums2013 at gmail.com> wrote:
>>>>
>>>> Hello,
>>>>
>>>> First time user of the webrev tool - I happened to use it today for one
>>>> of the patches I propose for a bug fix. In the generated webrev html
>>>> index file, I see that there's a "Workspace:" whose value points to the
>>>> local filesystem path of my system. I looked at the options available
>>>> for the webrev.ksh script (and even the code in the script) but couldn't
>>>> find a way to disable generating this (relatively private) information
>>>> in the webrev.
>>>>
>>>> Is there a reason to have this information exposed in the webrev? If
>>>> not, can a option be added to disable generating this?
>>>>
>>>> -Jaikiran
>>> It seems to me that this is a relic from a time where all users of webrev was located on the same filesystem. Today I would expect most users to run webrev on a local filesystem and the path to the workspace (and also the path to webrev at the bottom of the page) are of less value. I filed CODETOOLS-7902478 to add a flag to disable this output. The patch below fixes it. Please review.
>>>
>>> Thanks
>>> /Jesper
>>>
>>>
>>> diff --git a/webrev.ksh b/webrev.ksh
>>> old mode 100644
>>> new mode 100755
>>> --- a/webrev.ksh
>>> +++ b/webrev.ksh
>>> @@ -1812,6 +1812,7 @@
>>> -u <username>: Use that username instead of 'guessing' one.
>>> -m: Forces the use of Mercurial
>>> -C: Don't show comments
>>> + -w: Don't display local paths to workspace or webrev installation
>>>
>>> Mercurial only options:
>>> -r rev: Compare against a specified revision
>>> @@ -1904,7 +1905,8 @@
>>> Nflag=
>>> Cflag=
>>> forestflag=
>>> -while getopts "c:i:o:p:r:u:mONvfbC" opt
>>> +print_paths=1
>>> +while getopts "c:i:o:p:r:u:mONvfbCw" opt
>>> do
>>> case $opt in
>>> b) bflag=1;;
>>> @@ -1942,6 +1944,7 @@
>>>
>>> v) print "$0 version: $WEBREV_UPDATED";;
>>>
>>> + w) print_paths="";;
>>>
>>> ?) usage;;
>>> esac
>>> @@ -2706,7 +2709,9 @@
>>> print "<tr><th>Prepared by:</th><td>$username $date</td></tr>"
>>> fi
>>>
>>> +if [[ -n "$print_paths" ]]; then
>>> print "<tr><th>Workspace:</th><td>$CWS</td></tr>"
>>> +fi
>>> if [[ -n $parent_webrev ]]; then
>>> print "<tr><th>Compare against:</th><td>"
>>> print "webrev at $parent_webrev"
>>> @@ -2916,7 +2921,12 @@
>>> print
>>> print "<hr />"
>>> print "<p style=\"font-size: small\">"
>>> -print "This code review page was prepared using <b>$0</b>"
>>> +print "This code review page was prepared using "
>>> +if [[ -n "$print_paths" ]]; then
>>> + print "<b>$0</b>"
>>> +else
>>> + print "webrev"
>>> +fi
>>> print "(vers $WEBREV_UPDATED)."
>>> print "</body>"
>>> print "</html>"
>>>
>>>
More information about the webrev-dev
mailing list