Code review comment for lp:~abentley/bzr/is_executable

Revision history for this message
Andrew Bennetts (spiv) wrote :

The fix itself seems good.

(I already said this on IRC, but for the benefit of people looking at this page) The change to behaviour seems in principle reasonable, but I'd be nervous about landing it in the stable branch. I'm not sure how likely this is, but it's at least plausible that a plugin might be depending on the root entry being emitted in that case. So my preference is to land this just on lp:bzr.

One extremely minor nit is that the preferred way to do locking and addCleanup is now as a one-liner:

    self.addCleanup(tree.lock_read().unlock)

So if you wish, you could make that tweak, but it's not a big deal at all.

I'm going to vote "Abstain" because the key decision about whether to land this on 2.2 is up to the RM rather than a reviewer, I think.

review: Abstain

« Back to merge proposal