On Thu, Apr 25, 2013 at 3:14 AM, Daniel Shahaf <d...@daniel.shahaf.name> wrote: > Ivan Zhakov wrote on Thu, Apr 25, 2013 at 02:58:05 +0400: >> On Thu, Apr 25, 2013 at 2:01 AM, <danie...@apache.org> wrote: [...]
> > *nod*. Is that your preference or the project-agreed style? > It's not project agreed style, but I remember some discussions on the list that one variable per line is better in general. >> > + SVN_ERR_ASSERT(repos_version->major == SVN_VER_MAJOR); >> > + SVN_ERR_ASSERT(fs_version->major == SVN_VER_MAJOR); >> > + >> > + minor = (repos_version->minor > fs_version->minor) >> > + ? repos_version->minor : fs_version->minor; >> It would great to implement svn_ver_compare() and use it here. This > > There is svn_ver__at_least() but I didn't want to use non-public > functions. svn_ver__at_least() has a little bit semantics: it compares using our compatibility rules. Btw I don't see problem use our library private function anyway. > > On IRC you also made a point about the "Config files" output (prints the > path to DB_CONFIG or to fsfs.conf). Do you think it should be removed > from 'svnadmin info'? From svn_fs.h (svn_fs_info_config_files)? My personal opinion that "Config files" are not useful in 'svnadmin info' and should be removed from output and API. In this case 'svnadmin info' output would be more regular. Another problem is long repository path: [[[ % $svnadmin create C:\ProgramData\Subversion\SuperRepo && $svnadmin info C:\ProgramData\Subversion\SuperRepo Path: C:\ProgramData\Subversion\SuperRepo Repository Format: 5 Filesystem Format: 6 Compatible With Version: 1.8.0 Repository Capabilities: mergeinfo Filesystem Type: fsfs FSFS Sharded: yes FSFS Shard Size: 4 FSFS Packed: yes Config File: C:\ProgramData\Subversion\SuperRepo\db\fsfs.conf ]]] 'Config File' is hard to scan visually IMHO. -- Ivan Zhakov CTO | VisualSVN | http://www.visualsvn.com