launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #32470
Re: [Merge] ~ruinedyourlife/launchpad:craft-build-webhook into launchpad:master
The changes look good, but its missing unit tests.
There should be webhook unit tests for snaps and charms builds, so it should hopefully be simple to use those as a base
Diff comments:
> diff --git a/lib/lp/services/webhooks/model.py b/lib/lp/services/webhooks/model.py
> index 77f96aa..c8c8619 100644
> --- a/lib/lp/services/webhooks/model.py
> +++ b/lib/lp/services/webhooks/model.py
> @@ -306,6 +312,8 @@ class WebhookSet:
> Webhook.distribution == target.distribution,
> Webhook.source_package_name == target.sourcepackagename,
> )
> + elif ICraftRecipe.providedBy(target):
This is super minor, but I'd add these CraftRecipe `elifs` close to the snaps, charms and other recipe types to keep a theme in the list
> + target_filter = Webhook.craft_recipe == target
> else:
> raise AssertionError("Unsupported target: %r" % (target,))
> return (
--
https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/485534
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad:craft-build-webhook into launchpad:master.
References