On Thu, 2013-11-28 at 20:36 +0100, Sebastian Luther wrote: > Am 28.11.2013 15:47, schrieb Brian Dolbec: > > I like this one much better, thank you. > > > > Could you rebase the 2 patches together, this one shows removing > > lines from the first. Also see inline comments below :) > > The removed lines are another large if-block that already existed. > This has been moved into _ignore_dependency() too. >
Hey, all the more reason to move it to a function, that would have been 4 copies of the same check otherwise. Now it can be edited in one place to change behavior, and do it consistently the same. > > > > On Thu, 2013-11-28 at 11:34 +0100, sebastianlut...@gmx.de wrote: > >> From: Sebastian Luther <sebastianlut...@gmx.de> ... > > Since slot_operator_rebuild is already set either true or false, > > would it be better if "not slot_operator_rebuild" is first in the > > return statement. If it resolves False, it will be an early out > > for the remaining checks. Personally I don't know which is the > > more common case. But if it resolve False more often, it will be a > > slight speed improvement. That goes for the rest of them too, they > > should be ordered in the most often to resolve False order. Same > > for the if as well. > > This should be more often False than True. recurse_satisfied in the > second block should be more often True than False. > > In fact there's some duplication between the two blocks. Feel free to > clean this up after it has been pushed. > OK, thanks, I'll look to see if it can be optimized a little more.
signature.asc
Description: This is a digitally signed message part
