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):
subversion\libsvn_client\client.h(748):typedef struct svn_client__copy_pair_t
subversion\libsvn_client\client.h(832):typedef struct svn_client__committables_t
subversion\libsvn_client\mergeinfo.h(40):typedef struct svn_client__merge_path_t
subversion\libsvn_delta\delta.h(50):typedef struct svn_txdelta__ops_baton_t {
subversion\libsvn_diff\diff.h(36):typedef struct svn_diff__node_t
svn_diff__node_t;
subversion\libsvn_diff\diff.h(37):typedef struct svn_diff__tree_t
svn_diff__tree_t;
subversion\libsvn_diff\diff.h(38):typedef struct svn_diff__position_t
svn_diff__position_t;
subversion\libsvn_diff\diff.h(39):typedef struct svn_diff__lcs_t
svn_diff__lcs_t;
subversion\libsvn_fs_x\changes.h(45):typedef struct
svn_fs_x__changes_t svn_fs_x__changes_t;
subversion\libsvn_fs_x\dag.h(65):typedef struct dag_node_t dag_node_t;
subversion\libsvn_fs_x\fs.h(143):typedef struct svn_fs_x__shared_txn_data_t
subversion\libsvn_fs_x\fs.h(166):typedef struct svn_fs_x__shared_data_t
subversion\libsvn_fs_x\fs.h(205):typedef struct svn_fs_x__dag_cache_t
svn_fs_x__dag_cache_t;
subversion\libsvn_fs_x\fs.h(211):typedef struct svn_fs_x__pair_cache_key_t
subversion\libsvn_fs_x\fs.h(223):typedef struct
svn_fs_x__representation_cache_key_t
subversion\libsvn_fs_x\fs.h(238):typedef struct svn_fs_x__window_cache_key_t
subversion\libsvn_fs_x\fs.h(252):typedef struct svn_fs_x__data_t
subversion\libsvn_fs_x\fs.h(427):typedef struct svn_fs_x__transaction_t
subversion\libsvn_fs_x\fs.h(446):typedef struct svn_fs_x__representation_t
subversion\libsvn_fs_x\fs.h(479):typedef struct svn_fs_x__noderev_t
subversion\libsvn_fs_x\fs.h(534):typedef struct svn_fs_x__dirent_t
subversion\libsvn_fs_x\fs.h(549):typedef struct svn_fs_x__change_t
subversion\libsvn_fs_x\fs_id.h(40):typedef struct
svn_fs_x__id_context_t svn_fs_x__id_context_t;
subversion\libsvn_fs_x\id.h(77):typedef struct svn_fs_x__id_t
subversion\libsvn_fs_x\index.h(60):typedef struct svn_fs_x__p2l_entry_t
subversion\libsvn_fs_x\index.h(321):typedef struct svn_fs_x__page_cache_key_t
subversion\libsvn_fs_x\low_level.h(130):typedef struct svn_fs_x__rep_header_t
subversion\libsvn_fs_x\noderevs.h(45):typedef struct
svn_fs_x__noderevs_t svn_fs_x__noderevs_t;
subversion\libsvn_fs_x\reps.h(50):typedef struct
svn_fs_x__reps_builder_t svn_fs_x__reps_builder_t;
subversion\libsvn_fs_x\reps.h(55):typedef struct svn_fs_x__reps_t
svn_fs_x__reps_t;
subversion\libsvn_fs_x\reps.h(59):typedef struct
svn_fs_x__rep_extractor_t svn_fs_x__rep_extractor_t;
subversion\libsvn_fs_x\reps.h(63):typedef struct svn_fs_x__reps_baton_t
subversion\libsvn_fs_x\rev_file.h(38):typedef struct
svn_fs_x__packed_number_stream_t
subversion\libsvn_fs_x\rev_file.h(45):typedef struct svn_fs_x__revision_file_t
subversion\libsvn_fs_x\string_table.h(45):typedef struct
string_table_builder_t string_table_builder_t;
subversion\libsvn_fs_x\string_table.h(48):typedef struct
string_table_t string_table_t;
subversion\libsvn_fs_x\temp_serializer.h(198):typedef struct
svn_fs_x__ede_baton_t
subversion\libsvn_fs_x\temp_serializer.h(225):typedef struct replace_baton_t
subversion\libsvn_ra\ra_loader.h(42):typedef struct svn_ra__vtable_t {
subversion\libsvn_ra_local\ra_local.h(43):typedef struct
svn_ra_local__session_baton_t
subversion\libsvn_ra_serf\blncache.h(41):typedef struct
svn_ra_serf__blncache_t svn_ra_serf__blncache_t;
subversion\libsvn_ra_serf\ra_serf.h(63):typedef struct
svn_ra_serf__session_t svn_ra_serf__session_t;
subversion\libsvn_ra_serf\ra_serf.h(66):typedef struct
svn_ra_serf__connection_t {
subversion\libsvn_ra_serf\ra_serf.h(264):typedef struct
svn_ra_serf__dav_props_t {
subversion\libsvn_ra_serf\ra_serf.h(407):typedef struct
svn_ra_serf__server_error_t svn_ra_serf__server_error_t;
subversion\libsvn_ra_serf\ra_serf.h(415):typedef struct svn_ra_serf__handler_t {
subversion\libsvn_ra_serf\ra_serf.h(539):typedef struct
svn_ra_serf__xml_context_t svn_ra_serf__xml_context_t;
subversion\libsvn_ra_serf\ra_serf.h(543):typedef struct
svn_ra_serf__xml_estate_t svn_ra_serf__xml_estate_t;
subversion\libsvn_ra_serf\ra_serf.h(617):typedef struct
svn_ra_serf__xml_transition_t {
subversion\libsvn_ra_svn\ra_svn.h(54):typedef struct
svn_ra_svn__stream_st svn_ra_svn__stream_t;
subversion\libsvn_ra_svn\ra_svn.h(71):typedef struct
svn_ra_svn__session_baton_t svn_ra_svn__session_baton_t;
subversion\libsvn_subr\cache.h(33):typedef struct svn_cache__vtable_t {
subversion\libsvn_subr\crypto.h(46):typedef struct svn_crypto__ctx_t
svn_crypto__ctx_t;
subversion\libsvn_subr\fnv1a.h(37):typedef struct
svn_fnv1a_32__context_t svn_fnv1a_32__context_t;
subversion\libsvn_subr\fnv1a.h(59):typedef struct
svn_fnv1a_32x4__context_t svn_fnv1a_32x4__context_t;
subversion\libsvn_subr\win32_xlate.h(30):typedef struct win32_xlate_t
win32_xlate_t;
subversion\libsvn_subr\x509.h(78):typedef struct _x509_buf {
subversion\libsvn_subr\x509.h(84):typedef struct _x509_name {
subversion\libsvn_subr\x509.h(90):typedef struct _x509_cert {
subversion\libsvn_subr\utf8proc\utf8proc.h(173):typedef struct
utf8proc_property_struct {
subversion\libsvn_wc\entries.h(85):typedef struct svn_wc__text_base_file_info_t
subversion\libsvn_wc\entries.h(93):typedef struct svn_wc__text_base_info_t
subversion\libsvn_wc\wc_db.h(128):typedef struct svn_wc__db_t svn_wc__db_t;
subversion\libsvn_wc\wc_db.h(191):typedef struct svn_wc__db_lock_t {
subversion\libsvn_wc\wc_db.h(957):typedef struct svn_wc__db_install_data_t
subversion\libsvn_wc\wc_db.h(1266):typedef struct
svn_wc__db_commit_queue_t svn_wc__db_commit_queue_t;
subversion\libsvn_wc\wc_db.h(1714):typedef struct
svn_wc__db_revert_list_copied_child_info_t {
subversion\libsvn_wc\wc_db_private.h(73):typedef struct svn_wc__db_wclock_t
subversion\libsvn_wc\wc_db_private.h(87):typedef struct svn_wc__db_wcroot_t {
subversion\svn\cl-log.h(45):typedef struct svn_cl__log_receiver_baton
subversion\svn\cl.h(130):typedef struct svn_cl__opt_state_t
subversion\svn\cl.h(254):typedef struct svn_cl__cmd_baton_t
subversion\svn\cl.h(346):typedef struct svn_cl__interactive_conflict_baton_t
subversion\svn\cl.h(350):typedef struct svn_cl__conflict_stats_t
svn_cl__conflict_stats_t;
subversion\mod_dav_svn\dav_svn.h(85):typedef struct dav_svn_repos {
subversion\mod_dav_svn\dav_svn.h(187):typedef struct dav_svn_root {
subversion\mod_dav_svn\dav_svn.h(730):typedef struct dav_svn__authz_read_baton
subversion\mod_dav_svn\dav_svn.h(898):typedef struct dav_svn__uri_info {
subversion\libsvn_fs_fs\fs.h(202):typedef struct fs_fs_shared_txn_data_t
subversion\libsvn_fs_fs\fs.h(225):typedef struct fs_fs_shared_data_t
subversion\libsvn_fs_fs\fs.h(264):typedef struct fs_fs_dag_cache_t
fs_fs_dag_cache_t;
subversion\libsvn_fs_fs\index.h(266):typedef struct svn_fs_fs__page_cache_key_t
subversion\libsvn_fs_fs\low_level.h(192):typedef struct svn_fs_fs__rep_header_t
subversion\libsvn_fs_fs\rev_file.h(38):typedef struct
svn_fs_fs__packed_number_stream_t
subversion\libsvn_fs_fs\rev_file.h(45):typedef struct svn_fs_fs__revision_file_t
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)
> I wouldn't want to downplay HACKING, but surely we
> can make some changes every 15 years or so? At least to make HACKING
> match reality, instead of expecting things the other way around. :)
>
We can make changes in HACKING document of course, but we need to
discuss it first and then update code to match new code style. But
reaility that most code comply with our current guidelines.
--
Ivan Zhakov
Received on 2015-02-02 17:37:42 CET