On Mon, Feb 2, 2015 at 5:34 PM, Ivan Zhakov <i...@visualsvn.com> wrote:
> On 2 February 2015 at 19:17, Branko Čibej <br...@wandisco.com> wrote: > > On 02.02.2015 17:09, Ivan Zhakov wrote: > >> On 2 February 2015 at 18:55, Branko Čibej <br...@wandisco.com> wrote: > >>> On 02.02.2015 16:33, Ivan Zhakov wrote: > >>>> On 2 February 2015 at 18:24, <br...@apache.org> wrote: > >>>>> Author: brane > >>>>> Date: Mon Feb 2 15:24:16 2015 > >>>>> New Revision: 1656488 > >>>>> > >>>>> URL: http://svn.apache.org/r1656488 > >>>>> Log: > >>>>> Introduce a private libsvn_client context structure that stores > >>>>> context information that should not be part of the public API. > >>>>> > >>>>> * subversion/include/svn_client.h > >>>>> (svn_client_ctx_t): Remove the 'progress' field, which should be > private. > >>>>> > >>>>> * subversion/libsvn_client/client.h > >>>>> (client_ctx_t): New; the private context struct. > >>>>> Contains the equivalent of the 'progress' field. > >>>> I suggest svn_client__private_ctx_t name for client_ctx_t structure: I > >>>> think it less confusing name and follow our guidelines for library > >>>> private identifiers. > >>> This is not really a library private identifier. It is a type name > >>> private to the library, so it's never exposed from a (static or > dynamic) > >>> library and therefore cannot cause name collisions at link time. > >>> > >>> We use similar type and macro naming shortcuts in many places for this > >>> kind of thing. > >>> > >>> I'll probably add a "private_" prefix, though, to avoid confusion where > >>> both context types are used in the same function. > >>> > >> I know that there is no name collision problem with type names. But > >> Subversion Community Guide still require library prefix even for > >> structure names [1]: > >> [[[ > >> All published functions, variables, and structures must be signified > >> with the corresponding library name - such as libsvn_wc's > >> svn_wc_adm_open. All library-internal declarations made in a > >> library-private header file (such as libsvn_wc/wc.h) must be signified > >> by two underscores after the library prefix (such as > >> svn_wc__ensure_directory). > >> ]]] > >> > >> I think that svn_client__private_ctx_t structure name will be clearer > >> for readers since it gives a context where this type come from. > > > > Well, here's the thing: we don't use this convention for type names in > > many, many places. > It isn't true actually. Most modules follow our guidelines and > libsvn_client one of them (sorry for long list): > [...] > And only libsvn_fs_* don't follow it: > subversion\libsvn_fs\fs-loader.h(68):typedef struct fs_library_vtable_t > subversion\libsvn_fs\fs-loader.h(201):typedef struct fs_vtable_t > subversion\libsvn_fs\fs-loader.h(272):typedef struct txn_vtable_t > subversion\libsvn_fs\fs-loader.h(298):typedef struct root_vtable_t > subversion\libsvn_fs\fs-loader.h(426):typedef struct history_vtable_t > subversion\libsvn_fs\fs-loader.h(436):typedef struct id_vtable_t > subversion\libsvn_fs_base\dag.h(71):typedef struct dag_node_t dag_node_t; > subversion\libsvn_fs_base\fs.h(90):typedef struct base_fs_data_t > subversion\libsvn_fs_base\fs.h(122):typedef struct revision_t > subversion\libsvn_fs_base\fs.h(142):typedef struct transaction_t > subversion\libsvn_fs_base\fs.h(170):typedef struct node_revision_t > subversion\libsvn_fs_base\fs.h(234):typedef struct rep_delta_chunk_t > subversion\libsvn_fs_base\fs.h(259):typedef struct representation_t > subversion\libsvn_fs_base\fs.h(312):typedef struct copy_t > subversion\libsvn_fs_base\fs.h(330):typedef struct change_t > subversion\libsvn_fs_base\fs.h(349):typedef struct lock_node_t > subversion\libsvn_fs_base\trail.h(163):typedef struct trail_t trail_t; > subversion\libsvn_fs_base\bdb\env.h(47):typedef struct bdb_env_t bdb_env_t; > subversion\libsvn_fs_base\bdb\env.h(51):typedef struct bdb_error_info_t > subversion\libsvn_fs_base\bdb\env.h(78):typedef struct bdb_env_baton_t > subversion\libsvn_fs_fs\dag.h(64):typedef struct dag_node_t dag_node_t; > subversion\libsvn_fs_fs\fs.h(270):typedef struct pair_cache_key_t > subversion\libsvn_fs_fs\fs.h(282):typedef struct window_cache_key_t > subversion\libsvn_fs_fs\fs.h(296):typedef struct fs_fs_data_t > subversion\libsvn_fs_fs\fs.h(479):typedef struct transaction_t > subversion\libsvn_fs_fs\fs.h(502):typedef struct representation_t > subversion\libsvn_fs_fs\fs.h(560):typedef struct node_revision_t > subversion\libsvn_fs_fs\fs.h(610):typedef struct change_t > subversion\libsvn_fs_fs\temp_serializer.h(241):typedef struct > replace_baton_t > > (Most of structures in libsvn_fs_fs was originaly defined in .c files btw) > Not true. The *_cache_key_t are new identifiers while all others have been in those headers since the initial FSFS release in 1.1.0. -- Stefan^2.
