RFR: CODETOOLS-7902478 - Add webrev flag to avoid printing workspace (Was: Ability to not display local filesystem path in the generated webrev?)

Jaikiran Pai jai.forums2013 at gmail.com
Thu Jun 13 03:57:50 UTC 2019


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