> 1. You're using tabs. We don't use tabs in the Subversion code base
Oops, my bad. Badly configured emacs. Should be resolved in this resubmit.
> 2. The access types enum should be in svn_repos.h, not svn_types.h.
> There's no rule that says all types are defined in svn_types.h,
> despite the name.
Moved. When should types be defined in svn_types.h ?
> 3. I'm dubious about AUTHZ_ACCESS_GRANTED and AUTHZ_ACCESS_DETERMINED
> being defined as macros.
Changed to functions. The code is a little more spread out in the
functions; that helps readability.
> 4. [...] I think introducing a no-space-before-paren file into
> libsvn_repos would generate a lot of confusion.
Changed to space-before-paren.
> 5. I can't tell what changes you made between the old mod_authz_svn
> code and the new libsvn_repos code. Clearly there are some
> substantial changes, since the macros I mentioned in (3) above don't
> exist in the old code. Your log message should provide some
> guidance.
As stated in the first sentence of the commit log message, I reworked
the code to attempt to get it to fit with the svn coding style. This
implied a few variable renamings, renaming the functions to fit in the
svn namespace, commenting the code, etc. . Aside from the "cosmetic"
changes, the two macros are the only logical change I can recall.
The macros aren't in mod_authz_svn because the expansion is directly
used in a few places. I found this confusing (and spent a fair amount of
time working out why the hell they did what they did), and so moved it
to documented macros with a nice names. Well, documented functions now.
How can I explain all this in the commit message? I thought that for the
purposes of the commit message stating that the functionality has moved
and has been globally revamped to fit in with the destination was
sufficient; the comments in the new file are, I think, in sufficient
number to clear up any residual problems.
Anyhow, here's another attempt at the commit message, along with the
shiny, corrected patch.
[[[
Move authz routines from mod_authz_svn to libsvn_repos; rework authz
code to fit in with libsvn_repos; update mod_authz_svn to reflect
the move.
* subversion/mod_authz_svn/mod_authz_svn.c: Add includes, remove
nameless enum. Update copyright notice.
(parse_authz_baton, group_contains_user_internal,
group_contains_user, parse_authz_line,
parse_authz_lines, parse_authz_section,
parse_authz_sections, check_access): Remove, functionality
moves to libsvn_repos.
(req_check_access): Use the authz routines in libsvn_repos and wrap
any errors reported by the authz layer.
* subversion/include/svn_repos.h: Add include. Update copyright notice.
(svn_authz_access_kind_t): New enum type.
(svn_repos_authz_check_access): New public API.
* subversion/libsvn_repos/authz.c: New file. Contains the functionality
moved from mod_authz_svn. Rename functions and variable names to
better fit libsvn_repos. Add internal API and in-function
documentation.
* subversion/tests/libsvn_repos/repos-test.c: Add includes. Update
copyright notice.
(authz): New test.
]]]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Jul 3 03:37:57 2005