RE: svn commit: r1723876 - in /subversion/branches/fs-node-api/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h libsvn_fs/node_compat.c libsvn_fs_fs/node.c

See inline review below of a minor issue I found when this was committed, but didn't mail about then.

The more interesting question is: what is the status of this branch?

I would like to see this merged to trunk, unless there are some new problems.

> -----Original Message-----
> From: kotkov_at_apache.org [mailto:kotkov_at_apache.org]
> Sent: zaterdag 9 januari 2016 19:59
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1723876 - in /subversion/branches/fs-node-
> api/subversion: include/svn_fs.h libsvn_fs/fs-loader.c libsvn_fs/fs-loader.h
> libsvn_fs/node_compat.c libsvn_fs_fs/node.c
>
> Author: kotkov
> Date: Sat Jan 9 18:58:48 2016
> New Revision: 1723876
>
> URL: http://svn.apache.org/viewvc?rev=1723876&view=rev
> Log:
> On 'fs-node-api' branch: Allow getting the filesystem to which a particular
> filesystem node belongs. This is a prerequisite for switching the remaining
> parts of svn_ra_local__get_dir() to the new FS node API.
>
> * subversion/include/svn_fs.h
> (svn_fs_node_fs): New function.
>
> * subversion/libsvn_fs/fs-loader.h
> (struct svn_fs_node_t): Add 'fs' member to this structure.

I would say the relation is +-:

Fs -> root -> node.

In that case I would say the node should hold a pointer to root (which already holds a pointer to fs).

Why add a direct pointer 2 levels up, when the step via root is more obvious and probably already available.

(This is all about the member variable... I don't see a problem with the accessor function. It might not be needed later on after more cleanup as callers most likely already have all/most these pointers itself)

        Bert
Received on 2016-01-27 13:07:39 CET