Code review comment for lp:~mnn282/bzr/auto-rename-fix

Revision history for this message
Martin Packman (gz) wrote :

Thanks for working on this!

I'm still a little uncertain on the core logic here, will look again later.

+ if self.tree.has_filename(self.tree.id2path(parent[0])) == False:

Normal style is not to compare against booleans, just use `if not condition` instead.

+ self.assertEqual('added:\n folder/\nrenamed:\n file => folder/file2\n',
+ self.run_bzr("status")[0])

At this level, in bzrlib.tests rather than bzrlib.tests.blackbox, you really want to be writing assertions based on api calls rather than the commandline ui. Mostly this just confirms the check above, but adding an assertion for the current tree shape does seem reasonable.

I see you've already fixed the other style stuff I mentioned on IRC, thanks!

review: Approve

« Back to merge proposal