launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> === added file 'database/schema/patch-2209-83-6.sql'
> --- database/schema/patch-2209-83-6.sql	1970-01-01 00:00:00 +0000
> +++ database/schema/patch-2209-83-6.sql	2019-02-05 11:54:23 +0000
> @@ -0,0 +1,49 @@
> +-- Copyright 2019 Canonical Ltd.  This software is licensed under the
> +-- GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +SET client_min_messages=ERROR;
> +
> +CREATE TABLE BaseSnap (

Can we call this SnapBase to keep things vaguely together? The actual payload of a base is in a base snap, but a snap's base controls snapcraft and other behaviour in addition to selecting a base snap. So the reversed term is appropriate, in addition to sorting sensibly.

> +    id serial PRIMARY KEY,
> +    date_created timestamp without time zone DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC') NOT NULL,
> +    registrant integer NOT NULL REFERENCES person,
> +    name text NOT NULL,
> +    display_name text NOT NULL,
> +    distro_series integer NOT NULL REFERENCES distroseries,
> +    channels text NOT NULL,

It's not entirely clear that this isn't some list of channels of this snap. snap_channels, possibly?

> +    is_default boolean NOT NULL,

LP style (except in Translations) is usually to avoid the is_ prefix, but it may be reasonable to change that.

> +    CONSTRAINT valid_name CHECK (valid_name(name))
> +);
> +
> +CREATE UNIQUE INDEX basesnap__name__key ON BaseSnap (name);
> +CREATE INDEX basesnap__registrant__idx ON BaseSnap (registrant);
> +CREATE UNIQUE INDEX basesnap__is_default__idx ON BaseSnap (is_default) WHERE is_default;
> +
> +COMMENT ON TABLE BaseSnap IS 'A base snap.';
> +COMMENT ON COLUMN BaseSnap.date_created IS 'The date on which this base snap was created in Launchpad.';
> +COMMENT ON COLUMN BaseSnap.registrant IS 'The user who registered this base snap.';
> +COMMENT ON COLUMN BaseSnap.name IS 'The unique name of this base snap.';
> +COMMENT ON COLUMN BaseSnap.display_name IS 'The display name of this base snap.';
> +COMMENT ON COLUMN BaseSnap.distro_series IS 'The distro series used for snap builds that specify this base snap.';
> +COMMENT ON COLUMN BaseSnap.channels IS 'A dictionary mapping snap names to channels to use when building snaps that specify this base snap.';
> +COMMENT ON COLUMN BaseSnap.is_default IS 'Whether this base snap indicates the defaults used for snap builds that do not specify a base snap.';
> +
> +-- Allow defining snap recipes that detect the distro series from
> +-- snapcraft.yaml.
> +ALTER TABLE Snap ALTER COLUMN distro_series DROP NOT NULL;
> +
> +-- Allow combined vocabularies of (store_series, distro_series) pairs to
> +-- include entries for a store series without a distro series.  Columns that
> +-- are part of a primary key cannot be NULL, so replace the natural primary
> +-- key with a surrogate.
> +ALTER TABLE SnappyDistroSeries
> +    ADD COLUMN id serial,
> +    DROP CONSTRAINT snappydistroseries_pkey,
> +    ADD PRIMARY KEY (id),
> +    ALTER COLUMN distro_series DROP NOT NULL;
> +CREATE UNIQUE INDEX snappydistroseries__snappy_series__distro_series__idx
> +    ON SnappyDistroSeries (snappy_series, distro_series);
> +CREATE UNIQUE INDEX snappydistroseries__snappy_series__guess_distro_series__idx
> +    ON SnappyDistroSeries (snappy_series) WHERE distro_series IS NULL;

I'd explain what on earth SnappyDistroSeries.distro_series IS NULL means. It means autodetection, right?

> +
> +INSERT INTO LaunchpadDatabaseRevision VALUES (2209, 83, 6);


-- 
https://code.launchpad.net/~cjwatson/launchpad/db-base-snap/+merge/362727
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/db-base-snap into lp:launchpad/db-devel.