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