launchpad-reviewers team mailing list archive

[Ines Almeida <mp+493939@xxxxxxxxxxxxxxxxxx>]

Content looks good, left a few minor comments only

Diff comments:

> diff --git a/database/schema/patch-2211-48-0.sql b/database/schema/patch-2211-48-0.sql
> new file mode 100644
> index 0000000..f482b69
> --- /dev/null
> +++ b/database/schema/patch-2211-48-0.sql
> @@ -0,0 +1,15 @@
> +-- Copyright 2019 Canonical Ltd.  This software is licensed under the

Should be 2025

> +-- GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +SET client_min_messages=ERROR; 
> +
> +ALTER TABLE Webhook ADD COLUMN archive integer REFERENCES archive; 
> +
> +CREATE INDEX webhook__archive__id__idx 
> +    ON Webhook (archive, id) WHERE archive IS NOT NULL; 
> +
> +ALTER TABLE Webhook DROP CONSTRAINT one_target; 
> +ALTER TABLE Webhook ADD CONSTRAINT one_target 
> +    CHECK ( (public.null_count(ARRAY[git_repository, branch, snap, livefs, oci_recipe, charm_recipe, rock_recipe, craft_recipe, project, distribution, archive]) = 10) AND (source_package_name IS NULL OR distribution IS NOT NULL) ); 

nit: the code looks good, but I'd update the formatting to make it easier to read instead of having it all in one line.

There isn't a format we follow religiously for this afaik, but at least having different lines separated by the AND make it easier to understand there are 2 things it's checking IMO

(tbh I don't like how the previous couple of patches do it either. So I went back and found one I liked (2211-19-0) which turned out to be a change I did lol)

> +
> +INSERT INTO LaunchpadDatabaseRevision VALUES (2211, 48, 0);
> \ No newline at end of file

nit: can you add the extra line at the end of the file?



-- 
https://code.launchpad.net/~vaishnavi-asawale/launchpad/+git/launchpad/+merge/493939
Your team Launchpad code reviewers is requested to review the proposed merge of ~vaishnavi-asawale/launchpad:archive-webhooks-patch into launchpad:db-devel.