Another new-in-1.7 API.  A rather trivial one, this, and I don't want
this to be a long bikeshed discussion, but just looking at usage of the
new svn_error_return() macro, typified by these examples ...

  cmt_err = svn_error_return(
              check_nonrecursive_dir_delete(ctx->wc_ctx, ...));

  return svn_error_return(
           svn_ra_check_path(cukb->session, ...));

  return svn_error_return(err);

... I think the word "return" sounds out of place in the first example
and a bit redundant in the second and third examples.  The usage might
read better as ...

  cmt_err = svn_error_trace(
              check_nonrecursive_dir_delete(ctx->wc_ctx, ...));

  return svn_error_trace(
           svn_ra_check_path(cukb->session, ...));

  return svn_error_trace(err);

Any objections to me doing a global search an replace?  It's code churn,
I know, but since it's code that every developer sees everywhere, I care
enough to change it if we agree to do so.

We could go one step further and define a statement-macro similar to
SVN_ERR for the actual "return" statements, like this ...

  #define SVN_ERR_RETURN(err) return svn_error_trace(err)

  cmt_err = svn_error_trace(
              check_nonrecursive_dir_delete(ctx->wc_ctx, ...));

  SVN_ERR_RETURN(svn_ra_check_path(cukb->session, ...));

  SVN_ERR_RETURN(err);

I'm not currently proposing that, I'm just throwing it out as a related
idea, partly to show how the macro currently called "svn_error_return"
would then be mostly confined to non-return contexts.

- Julian


Reply via email to