[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: initial attempt at a modrdn fix



Great work!

> I also found the cache and locking system interface to
> be awkward

Don't we all...  We are hoping that someone will stare at the cache
code for long enough that she manages to document it:-)

> Also, i've had to move functions around between modules/source files,
> and am not sure I did it in the appropriate manner.

Would it work just as good to simply remove the "static" keyword and
move the declarations to proto-slapd.h or back-ldbm/proto-back-ldbm.h?
Small patches are easier to review and manage than big ones.  If not, a
"guide to the patch" would be nice: Which changes only move unchanged
functions around, and which ones are "real changes".

> This patch also adds support to:
> 	- add the new rdn attribute/value
> 	- delete the old rdn attribute/value if asked to do so
> 	- update the lastmod attributes
> 	- update indexes for the above
> 	- test the modrdn changes in the test suite

It would be _much_ easier to review the code if you submit each change
as a separate patch.  One for the bug fix, one for the new features - or
one for each new feature.  Or if the fixes depend on some of the new
featuers, add the features first, of course.  (And say in which order
the patches should be applied.)

Detail:  The diff is reversed.  The command should be
	diff {-options} old new
not	diff {-options} new old
It's not a problem, since `patch' detects reversed patches,
non-reversed patches are a little easier to read.

...oh, and send it with a mailer which doesn't break lines.
Or send it as base64 or uuencoded, so there are no lines to break.
Or put it up for FTP or HTTP and publish its URL instead.

-- 
Hallvard