On Mon, Feb 2, 2015 at 5:34 PM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 2 February 2015 at 19:17, Branko Čibej <brane_at_wandisco.com> wrote:
> > On 02.02.2015 17:09, Ivan Zhakov wrote:
> >> On 2 February 2015 at 18:55, Branko Čibej <brane_at_wandisco.com> wrote:
> >>> On 02.02.2015 16:33, Ivan Zhakov wrote:
> >>>> On 2 February 2015 at 18:24, <brane_at_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.
Received on 2015-02-02 17:52:49 CET