Eric Gillespie <epg@pretzelnet.org> writes:
> You didn't address my point. Documenting this one error code in
> these few functions doesn't really help. Someone looking to see
> which error codes any of these functions returns will still have
> to read the code since no one has gone through documenting all
> the error codes yet. Someone going through to document all those
> error codes will still have to read the code to do the actual
> documenting.
Yes, I did address your point :-), or at least I meant to.
Your point, as I understand it, is: What good does it do to document
this one error, when the function can return N different errors, and
N-1 of them will still be undocumented?
My answer: When a caller *depends* on (i.e., tests for) a particular
error code, then it's binding the callee to a contract, and the
callee's doc string therefore needs to describe that promise, the
circumstances in which it returns that particular error code.
Although that only slightly improves the situation for someone reading
the callee's documentation, it *greatly* improves the situation for
someone reading the caller and wondering what circumstance the error
code in question represents.
This isn't some new requirement that I just made up (though admittedly
it doesn't seem to be in HACKING). It's been consistently pointed out
in code reviews since the very beginning of the project, and as far as
I know everyone follows this convention. When they don't, someone
(and not necessarily me) always points it out.
Look, don't take my word for it. Grep for "SVN_ERR_" to see places
where we test specific error codes -- you'll find very few cases where
those particular codes are not documented by their producers, even
when their producers can also return other errors that are not
documented.
> > Our practice has been, if some caller tests for a specific error, then
> > that particular error should be documented in the callee, and all the
> > way down as necessary.
>
> That's a pretty haphazard way to write documentation.
Sure, it would certainly be better for every function to document
every error code it could return. If we could do it over again,
that's probably what we'd do. This solution is a compromise; I never
said it wasn't.
> > (Most errors are never tested for explicitly, they're just handled at
> > the top level and the error string is thrown at the user.)
>
> That is the state today. In my experience, error codes usually
> end up consumed more by programmers making use of the interface
> in question than end users. We will see more and more testing
> for specific error codes as i have done as time goes on.
Eric, I'm not arguing *against* documenting every error that a
function could return. Everyone thinks that's a fine idea.
The only problem is we haven't been requiring it up till now.
However, we *have* been requiring a subset of that policy, the subset
that probably gets us the highest payoff/effort ratio.
You're certainly free to document more than just the error codes your
caller depends on. Just please don't document less than that.
> OK, how does this patch look? The diff is identical to the one i
> sent last time. The only difference is the addition of the first
> part, a change to libsvn_client/revert.c.
Looks good, except for the doc string thing, of course.
I wonder, what if we had a variant of the SVN_ERR,
SVN_ERR_GOTO(val, label)
that jumped to `label' if val is non-NULL?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Apr 29 17:11:29 2003