On Tue, May 15, 2012 at 01:16:44AM +0200, Dmitry Pavlenko wrote: > Hi Stefan, > > I'll have a look a bit later. But from the first glance I've found a couple > of suspicious places. I > do not state that they are wrong or that I have a counterexample, but just > not clear for me. >
Thanks much for the review! > 1. When you call callbacks->dir_props_changed for added properties, you > convert them to regular, but > for deleted properties you do not (and the same is true for file properties > diff). Ah, yes, you are right. We should filter deleted props, too. The diff_file_deleted() and diff_dir_deleted() callbacks currently don't display any deleted props, so this change won't affect existing behaviour. But that is an implementation detail of these callback and no business of the code that handles added/deleted diff targets. > 2. In diff_repos_repos_added_or_deleted_dir in "for" cycle you use different > pools (iterpool and > scratch_pool). Already fixed in r1338297. Note also that another tweak was made since: r1338314
