launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/lp/code/interfaces/branch.py b/lib/lp/code/interfaces/branch.py
> index f0e9393..ccb4084 100644
> --- a/lib/lp/code/interfaces/branch.py
> +++ b/lib/lp/code/interfaces/branch.py
> @@ -1427,10 +1427,36 @@ class IBranchSet(Interface):
>          Return None if no match was found.
>          """
>  
> -    @collection_default_content()
> -    def getBranches(limit=50, eager_load=True):
> +    @call_with(user=REQUEST_USER)
> +    @operation_parameters(
> +        order_by_modified_date=Bool(

We can use Choice with a suitable vocabulary - no need to fall back to TextLine.  I see the point about adding more order_by_* parameters being potentially cumbersome.

In fact, if we're doing that, we might as well use the existing BranchListingSort which is used for sorting branches in the UI.  There isn't currently an equivalent GitRepositoryListingSort, but it wouldn't be too hard to add ...

> +            title=_("Order by modified date"),
> +            # order_by_modified_date=False currently returns branches in the
> +            # same order as True (descending date_last_modified followed by
> +            # descending ID), but we don't want to commit to this.
> +            description=_(
> +                "Return most-recently-modified branches first.  If False, "
> +                "the order of results is unspecified."),
> +            required=False),
> +        modified_since_date=Datetime(
> +            title=_("Modified since date"),
> +            description=_(
> +                "Return only branches whose `date_last_modified` is "
> +                "greater than or equal to this date.")))
> +    @operation_returns_collection_of(IBranch)
> +    @export_read_operation()
> +    @operation_for_version("devel")
> +    @collection_default_content(user=None, limit=50)
> +    def getBranches(user, order_by_modified_date=False,
> +                    modified_since_date=None, limit=None, eager_load=True):
>          """Return a collection of branches.
>  
> +        :param user: An `IPerson`.  Only branches visible by this user will
> +            be returned.
> +        :param order_by_modified_date: If True, order results by descending
> +            `date_last_modified`; if False, the order is unspecified.
> +        :param modified_since_date: If not None, return only branches whose
> +            `date_last_modified` is greater than this date.
>          :param eager_load: If True (the default because this is used in the
>              web service and it needs the related objects to create links)
>              eager load related objects (products, code imports etc).


-- 
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/380391
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:date-ordered-repositories into launchpad:master.