Bhuvaneswaran Arumugam wrote:
>>>
>>>
>> Can you retain original namings like 'fs', 'rev', 'path' instead of 'f',
>> 'r' and 'p'.? Same with other macros too.
>>
>
> In most of cases if we use original naming convention, we face a
> compiler error as indicated in this link [1]. That is one of reason for
> me to change the variable names on most of occasion, assuming it does
> not harm and I also noted that this convention is followed elsewhere in
> the code base.
>
The cause of the error you see is the way macros work, i.e 'Find and
replace'.
<snip of your original macro with single char formal params>
#define SVN_FS__ERR_NOT_FILE(fs, path) \
svn_error_createf \
(SVN_ERR_FS_NOT_FILE, 0, \
_("'%s' is not a file in filesystem '%s'"), \
path, fs->path)
</snip>
If you call SVN_FS__ERR_NOT_FILE(a, b->c) it translates to
....
_("'%s' is not a file in filesystem '%s'"), \
b->c, a->b->c)
The following macro fixes it in a different way without compromising the
meaning of the variables.
#define SVN_FS__ERR_NOT_FILE(fs, path_inside_repo) \
svn_error_createf \
(SVN_ERR_FS_NOT_FILE, 0, \
_("'%s' is not a file in filesystem '%s'"), \
path_inside_repo, fs->path)
I personally feel descriptive names are of great use rather than f, p.
If some old code has 'f', 'p' they are precedants not the conventions.
>
> In this case, I did not explain the type of "fs" argument, as it is
> obvious! In addition, elsewhere in code base, I do not see any occasion
> where we explain the parameter for a macro when it is obvious.
>
Fine.
>
>>> -/* SVN_ERR_FS_LOCK_OWNER_MISMATCH: trying to use a lock whose LOCK_OWNER
>>> - doesn't match the USERNAME associated with FS. */
>>> -svn_error_t *svn_fs__err_lock_owner_mismatch(svn_fs_t *fs,
>>> - const char *username,
>>> - const char *lock_owner);
>>> +/* SVN_FS__ERR_LOCK_OWNER_MISMATCH: trying to use a lock whose
>>> + LOCK_OWNER doesn't match the USERNAME associated with FS.
>>> + F is of type "svn fs_t *", U is of type "const char *",
>>> + O is of type "const char *". */
>>>
>>>
>> Use the original function argument names. Here you have 'u', but doc
>> says 'USERNAME'.
>>
>
> The USERNAME explains the context for the parameter though the type for
> this argument is explained underneath.
>
> Let me know what you think. Thank you very much.
>
We use for our internal private APIs *ALL_CHARS_IN_UPPER_OF_PARAM*
style. Here you use *USERNAME* without having any equivalent parameter.
For a casual browser of code 'u' is not same as 'username'.
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Jun 7 16:09:10 2007