subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "C. Michael Pilato" <cmpil...@collab.net>
Subject Re: [PATCH] '--version --quiet' should display only the version info in svnadmin, svnlook, svnsync, svndumpfilter, svnversion, svnserve
Date Wed, 05 Jan 2011 14:37:15 GMT
On 01/05/2011 06:42 AM, Prabhu Gnana Sundar wrote:
> Hi all,
> 
> Currently svnadmin, svnlook, svnsync, svndumpfilter, svnversion,
> svnserve show the entire version information even if '--version --quiet'
> is given as command.
> 
> The '--quiet' option has *not* been handled so far.
> 
> This patch would make svnadmin, svnlook, svnsync, svndumpfilter,
> svnversion, svnserve '--version --quiet' to display *only* the version
> number(info) just like 'svn --version --quiet' does.

Mmm... consistency.  Consistency makes me smile.

> I have attached the patch and the log message with this mail. Please
> give your views on this.

I have reviewed this patch and will commit it shortly (after some testing).
 I made some small tweaks to it, which I'll detail below.

> Index: subversion/svnversion/main.c
> ===================================================================
> --- subversion/svnversion/main.c	(revision 1055432)
> +++ subversion/svnversion/main.c	(working copy)

[...]

> @@ -206,6 +212,11 @@
>          }
>      }
>  
> +  if(is_version)

Keep the space before the paren here:  if (is_version)

> Index: subversion/svnlook/main.c
> ===================================================================
> --- subversion/svnlook/main.c	(revision 1055432)
> +++ subversion/svnlook/main.c	(working copy)
> @@ -176,6 +176,9 @@
>                         "                            "
>                         "       Ignore changes in EOL style")},
>  
> +  {"quiet",         'q', 0,
> +   N_("no progress (only errors) to stderr")},
> +

There are some code formatting/indentation inconsistencies here which didn't
originate with your change, but which your change extends.

> Index: subversion/svnserve/main.c
> ===================================================================
> --- subversion/svnserve/main.c	(revision 1055432)
> +++ subversion/svnserve/main.c	(working copy)
> @@ -218,6 +218,8 @@
>      {"help",             'h', 0, N_("display this help")},
>      {"version",           SVNSERVE_OPT_VERSION, 0,
>       N_("show program version information")},
> +    {"quiet",         'q', 0,
> +     N_("no progress (only errors) to stderr")},

Oops.  More indentation inconsistencies!

> Index: subversion/svnrdump/svnrdump.c
> ===================================================================
> --- subversion/svnrdump/svnrdump.c	(revision 1055432)
> +++ subversion/svnrdump/svnrdump.c	(working copy)
> @@ -484,8 +484,10 @@
>   */
>  static svn_error_t *
>  version(const char *progname,
> +        void *baton,
>          apr_pool_t *pool)

In other tools, you tweaked the version() function to accept a boolean
'quiet' parameter.  Here, you accept a(n unnecessarily void *) baton.  Not
sure why you did, but I'm changing this to also accept the boolean.



-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Mime
View raw message