Code review comment for lp:~cjwatson/brz/fastimport-fix-directory-renames

Revision history for this message
Jelmer Vernooij (jelmer) wrote :

For --no-plain mode, it should indeed just emit a rename for the directory - the importer doesn't need to know about all the unchanged children.

I think that just means that you can change the condition on line 597 from:

 if change.kind == ('directory', 'directory'):

to

  if change.kind == ('directory', 'directory') and self.plain_format:

Iterating over the entire tree works, but is not very efficient :( Tree.list_files() is recursive by default and allows you to pass in a from_dir. Would it be possible to use that instead?

It would also be great to have a test if you can easily reproduce the situation in https://bazaar.launchpad.net/~lazr-developers/launchpadlib/trunk/revision/36.1.1; this code is quite subtle so otherwise there's risk that future changes reintroduce this bug.

« Back to merge proposal