launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lib/lp/snappy/browser/snap.py'
> --- lib/lp/snappy/browser/snap.py	2015-07-23 16:02:58 +0000
> +++ lib/lp/snappy/browser/snap.py	2015-08-07 10:52:53 +0000
> @@ -26,3 +34,54 @@
>          if build is None or build.snap != self.context:
>              return None
>          return build
> +
> +
> +class SnapBreadcrumb(NameBreadcrumb):
> +
> +    @property
> +    def inside(self):
> +        return Breadcrumb(
> +            self.context.owner,
> +            url=canonical_url(self.context.owner, view_name="+snap"),
> +            text="Snap packages", inside=self.context.owner)
> +
> +
> +class SnapView(LaunchpadView):
> +    """Default view of a Snap."""
> +
> +    @property
> +    def page_title(self):
> +        return "%(name)s's %(snap_name)s snap package" % {
> +            'name': self.context.owner.displayname,
> +            'snap_name': self.context.name,
> +            }
> +
> +    label = page_title

page_title and label are unused: you override the heading slot's use of label, and page_title isn't used for breadcrumbs on default views.

> +
> +    @property
> +    def builds(self):
> +        return builds_for_snap(self.context)
> +
> +
> +def builds_for_snap(snap):
> +    """A list of interesting builds.
> +
> +    All pending builds are shown, as well as 1-10 recent builds.  Recent
> +    builds are ordered by date finished (if completed) or date_started (if
> +    date finished is not set due to an error building or other circumstance
> +    which resulted in the build not being completed).  This allows started
> +    but unfinished builds to show up in the view but be discarded as more
> +    recent builds become available.
> +
> +    Builds that the user does not have permission to see are excluded.
> +    """
> +    builds = [
> +        build for build in snap.pending_builds
> +        if check_permission('launchpad.View', build)]
> +    for build in snap.completed_builds:
> +        if not check_permission('launchpad.View', build):
> +            continue
> +        builds.append(build)
> +        if len(builds) >= 10:
> +            break
> +    return builds


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-basic-browser/+merge/267324
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.