You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2013/01/09 20:36:11 UTC
Re: svn commit: r1431017 - in /subversion/trunk: subversion/libsvn_delta/ subversion/libsvn_fs_base/ subversion/libsvn_fs_fs/ subversion/libsvn_ra_serf/ subversion/libsvn_ra_svn/ subversion/libsvn_repos/ subversion/libsvn_subr/ subversion/svn/ subversion/s...
Stefan,
> Log:
> Coding style patch: in logical expressions, instead of comparing
> with logical constants use arithmetic operations. I.e. replace
No don't use arithmetic operations. Maybe write, "In *boolean* expressions, instead of comparing with FALSE (or, worse, with TRUE), use *boolean* operators."
> Modified: subversion/trunk/subversion/libsvn_ra_svn/client.c
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_svn/client.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_svn/client.c Wed Jan 9 19:24:52 2013
> @@ -1304,9 +1304,9 @@ static svn_error_t *ra_svn_get_dir(svn_r
> static svn_tristate_t
> optbool_to_tristate(apr_uint64_t v)
> {
> - if (v == TRUE)
> + if (v)
No, no, not here!
> return svn_tristate_true;
> - if (v == FALSE)
> + if (!v)
> return svn_tristate_false;
>
> return svn_tristate_unknown; /* Contains SVN_RA_SVN_UNSPECIFIED_NUMBER */
Everything else looks correct.
- Julian
Re: svn commit: r1431017 - in /subversion/trunk: subversion/libsvn_delta/
subversion/libsvn_fs_base/ subversion/libsvn_fs_fs/ subversion/libsvn_ra_serf/
subversion/libsvn_ra_svn/ subversion/libsvn_repos/ subversion/libsvn_subr/
subversion/svn/ subversion/s...
Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Wed, Jan 9, 2013 at 8:36 PM, Julian Foad <ju...@btopenworld.com>wrote:
> Stefan,
>
> > Log:
>
> > Coding style patch: in logical expressions, instead of comparing
> > with logical constants use arithmetic operations. I.e. replace
>
> No don't use arithmetic operations. Maybe write, "In *boolean*
> expressions, instead of comparing with FALSE (or, worse, with TRUE), use
> *boolean* operators."
>
> The wording should be slightly better now.
>
> > Modified: subversion/trunk/subversion/libsvn_ra_svn/client.c
> >
> ==============================================================================
> > --- subversion/trunk/subversion/libsvn_ra_svn/client.c (original)
> > +++ subversion/trunk/subversion/libsvn_ra_svn/client.c Wed Jan 9
> 19:24:52 2013
> > @@ -1304,9 +1304,9 @@ static svn_error_t *ra_svn_get_dir(svn_r
> > static svn_tristate_t
> > optbool_to_tristate(apr_uint64_t v)
> > {
> > - if (v == TRUE)
> > + if (v)
>
> No, no, not here!
>
You beat me by 2 minutes ;)
> > return svn_tristate_true;
> > - if (v == FALSE)
> > + if (!v)
> > return svn_tristate_false;
> >
> > return svn_tristate_unknown; /* Contains
> SVN_RA_SVN_UNSPECIFIED_NUMBER */
>
> Everything else looks correct.
>
Thanks for the review!
-- Stefan^2.
--
Certified & Supported Apache Subversion Downloads:
*
http://www.wandisco.com/subversion/download
*