launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Hi Abel

There's nothing wrong with being paranoid :-)
I didn't think it is possible to make circular stacked on branches but 
I do think your suggestion is good just in case. I'll redo that bit of 
my code.

Thanks.

On Fri 13 Jul 2012 20:24:24 EST, Abel Deuring wrote:
> Review: Approve code
>
> Overall, this looks good..
>
> Perhaps I am simply paranoid (and I couldn't find the tinfoil hat today...), but anyway: I feel a bit uncomfortable about this:
>
> +    def getStackedOnBranches(self):
> +        """See `IBranch`."""
> +        if not self.stacked_on:
> +            return []
> +        stacked_on_branches = [self.stacked_on]
> +        stacked_on_branches.extend(self.stacked_on.getStackedOnBranches())
> +        return stacked_on_branches
>
> Sure, the stacked_on relation should not be circular -- but we are working on "DB copy representation" of the original Bazaar relation, which might have its own bugs, and Bazaar might also (probably inadvertently) allow circular stacked_on relations. And this would lead to an infinite loop. What about something like this:
>
> stacked_on_branches = []
> branch = self
> while branch.stacked_on and branch.stacked_on not in stacked_on_branches:
>     stacked_on_branches.append(branch.stacked_on)
>     branch = branch.stacked_on
> return stacked_on_branches
>
>

-- 
https://code.launchpad.net/~wallyworld/launchpad/view-private-stacked-branch-393566/+merge/114784
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.