launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lib/lp/code/model/sourcepackagerecipedata.py'
> --- lib/lp/code/model/sourcepackagerecipedata.py	2016-01-12 01:39:14 +0000
> +++ lib/lp/code/model/sourcepackagerecipedata.py	2016-01-12 01:39:14 +0000
> @@ -329,44 +397,72 @@
>  
>      @staticmethod
>      def preLoadReferencedBranches(sourcepackagerecipedatas):
> -        # Circular import.
> +        # Circular imports.
>          from lp.code.model.branchcollection import GenericBranchCollection
> +        from lp.code.model.gitcollection import GenericGitCollection
>          # Load the related Branch, _SourcePackageRecipeDataInstruction.
>          base_branches = load_related(
>              Branch, sourcepackagerecipedatas, ['base_branch_id'])
> +        base_repositories = load_related(
> +            GitRepository, sourcepackagerecipedatas,
> +            ['base_git_repository_id'])
>          sprd_instructions = load_referencing(
>              _SourcePackageRecipeDataInstruction,
>              sourcepackagerecipedatas, ['recipe_data_id'])
>          sub_branches = load_related(
>              Branch, sprd_instructions, ['branch_id'])
> +        sub_repositories = load_related(
> +            GitRepository, sprd_instructions, ['git_repository_id'])
>          all_branches = base_branches + sub_branches
> -        # Pre-load branches' data.
> -        GenericBranchCollection.preloadDataForBranches(all_branches)
> +        all_repositories = base_repositories + sub_repositories
> +        # Pre-load branches'/repositories' data.
> +        if all_branches:
> +            GenericBranchCollection.preloadDataForBranches(all_branches)
> +        if all_repositories:
> +            GenericGitCollection.preloadDataForRepositories(all_repositories)
>          # Store the pre-fetched objects on the sourcepackagerecipedatas
>          # objects.
> -        branch_to_recipe_data = dict([
> -            (instr.branch_id, instr.recipe_data_id)
> -                for instr in sprd_instructions])
> -        caches = dict((sprd.id, [sprd, get_property_cache(sprd)])
> -            for sprd in sourcepackagerecipedatas)
> +        branch_to_recipe_data = {
> +            instr.branch_id: instr.recipe_data_id
> +            for instr in sprd_instructions
> +            if instr.branch_id is not None}
> +        repository_to_recipe_data = {
> +            instr.git_repository_id: instr.recipe_data_id
> +            for instr in sprd_instructions
> +            if instr.git_repository_id is not None}
> +        caches = {
> +            sprd.id: [sprd, get_property_cache(sprd)]
> +            for sprd in sourcepackagerecipedatas}
>          for unused, [sprd, cache] in caches.items():
> -            cache._referenced_branches = [sprd.base_branch]
> +            cache._referenced_branches = [sprd.base]
>          for recipe_data_id, branches in groupby(
> -            sub_branches, lambda branch: branch_to_recipe_data[branch.id]):
> +                sub_branches, lambda branch: branch_to_recipe_data[branch.id]):
>              cache = caches[recipe_data_id][1]
>              cache._referenced_branches.extend(list(branches))
> +        for recipe_data_id, repositories in groupby(
> +                sub_repositories,
> +                lambda repository: repository_to_recipe_data[repository.id]):
> +            cache = caches[recipe_data_id][1]
> +            cache._referenced_branches.extend(list(repositories))

I'm not sure either groupby is useful. The chance of things coming back in order is low, and it doesn't obviously save any time.

>  
>      def getReferencedBranches(self):
> -        """Return an iterator of the Branch objects referenced by this recipe.
> +        """Return an iterator of the Branch/GitRepository objects referenced
> +        by this recipe.
>          """
>          return self._referenced_branches
>  
>      @cachedproperty
>      def _referenced_branches(self):
> -        referenced_branches = [self.base_branch]
> +        referenced_branches = [self.base]
>          sub_branches = IStore(self).find(
>              Branch,
>              _SourcePackageRecipeDataInstruction.recipe_data == self,
>              Branch.id == _SourcePackageRecipeDataInstruction.branch_id)
>          referenced_branches.extend(sub_branches)
> +        sub_repositories = IStore(self).find(
> +            GitRepository,
> +            _SourcePackageRecipeDataInstruction.recipe_data == self,
> +            GitRepository.id ==
> +                _SourcePackageRecipeDataInstruction.git_repository_id)
> +        referenced_branches.extend(sub_repositories)
>          return referenced_branches


-- 
https://code.launchpad.net/~cjwatson/launchpad/git-recipe-model/+merge/282248
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.