Re: Review of APIs new in 1.2

[Julian Foad <ju...@btopenworld.com>]

Julian Foad wrote:
> Here is a quick review of the public APIs marked "@since New in 1.2" in 
> r13616, and of the doc strings of the APIs deprecated by them.

Here are the remaining items now that some have been postponed and many others 
fixed.


svn_wc_adm_open2
svn_wc_adm_open
svn_wc_adm_probe_open2
svn_wc_adm_probe_open
svn_wc_adm_probe_try2
svn_wc_adm_probe_try
   Bug: Bogus doc strings, referring to wrong parameters and sometimes to 
inappropriate later versions.


svn_client_move2
   Bug: parameter "force" needs a better name.  (Ensure that its deprecated 
predecessor describes the conversion of "force" to the new parameter.)


SVN_ERR_FS_PATH_LOCKED
SVN_ERR_FS_NO_LOCK_TOKEN
   Need to clarify the difference between these, or merge them.  To me they 
mean the same: the path is locked and no (correct) lock tocken has been supplied.

SVN_ERR_FS_OUT_OF_DATE
   Does this necessarily refer to a locking-related error? 
SVN_ERR_IS_LOCK_ERROR thinks it does, but its name and message do not imply so.

SVN_ERR_IS_LOCK_ERROR
SVN_ERR_IS_UNLOCK_ERROR
   Someone raised a concern about whether these belong in svn_error.h or are 
too specialised to be there.

SVN_ERR_* (in svn_error_codes.h)
   Bug (maybe): The comments should be Doxygen comments (starting "/**").


svn_fs_unlock
   Slight contradiction.  Before "return @c SVN_ERR_FS_LOCK_OWNER_MISMATCH", 
add "and @a break_lock is false,".


svn_fs_begin_txn2
   The description of "flags" is a tiny bit vague; it should state explicitly 
that the argument is to be composed from the mentioned constants.


(all public headers)
   Use of "true" and "false" versus "@c TRUE" etc.  I think we should say 
"true" and "false".
   Use of "<pre>", "note", "warning", "@note", "@warning", etc.
   Use of "<tt>" versus "@c".
   Use of "@a" to refer to args of a different function.


svn_ctype.h
svn_ra_reporter2_t
svn_wc_diff_callbacks2_t
   Not reviewed.



> SVN_ERR_REPOS_UNSUPPORTED_VERSION
> SVN_ERR_FS_UNSUPPORTED_FORMAT
>   Why has the error number changed?  [...]

I thought that this was one error whose name had changed from "*_VERSION" to 
"*_FORMAT", but now I know that these are two separate errors, so there is no 
problem with them.


- Julian


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Review of APIs new in 1.2

[Julian Foad <ju...@btopenworld.com>]

Julian Foad wrote:
>> svn_ctype.h
>> svn_ra_reporter2_t
>> svn_wc_diff_callbacks2_t
>>   Not reviewed.
> 
> If I review those I'll do so in a separate thread.

I've now reviewed those, and fixed some typos.  OK, so it wasn't worth a 
separate thread.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Review of APIs new in 1.2

[Julian Foad <ju...@btopenworld.com>]

Update as of r13909.

The items not mentioned here have already been resolved, and here I wrap up the 
loose ends:

Julian Foad wrote:
> svn_client_move2
>   Bug: parameter "force" needs a better name.

I have at last dug into history to read the deleted "notes/TODO-1.2-foad.txt" 
file and seen the comments in there - including that it was difficult to find a 
name for this last "force" parameter.  I might have a go at it; if not, it will 
just have to stay until another time.


> SVN_ERR_FS_PATH_LOCKED
> SVN_ERR_FS_NO_LOCK_TOKEN
>   Need to clarify the difference between these, or merge them.

I've started a separate thread about this.


> SVN_ERR_IS_LOCK_ERROR
> SVN_ERR_IS_UNLOCK_ERROR
>   Someone raised a concern about whether these belong in svn_error.h or 
> are too specialised to be there.

They can stay.


> SVN_ERR_* (in svn_error_codes.h)
>   Bug (maybe): The comments should be Doxygen comments (starting "/**").

I'm leaving that as I don't feel strongly enough about it.


> svn_fs_unlock
>   Slight contradiction.

That's not worth bothering with.  It's clear enough.


> (all public headers)
>   Use of "true" and "false" versus "@c TRUE" etc.  I think we should say 
> "true" and "false".
>   Use of "<pre>", "note", "warning", "@note", "@warning", etc.
>   Use of "<tt>" versus "@c".
>   Use of "@a" to refer to args of a different function.

Those, and more, are all aspects of our Doxygen usage policy which I will 
address in a separate thread.


> svn_ctype.h
> svn_ra_reporter2_t
> svn_wc_diff_callbacks2_t
>   Not reviewed.

If I review those I'll do so in a separate thread.


Therefore I consider this thread wrapped up.  Thanks for listening.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org