[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562 -- Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad
Review: Approve
Diff comments:
>
> === modified file 'lib/lp/code/browser/tests/test_branchlisting.py'
> --- lib/lp/code/browser/tests/test_branchlisting.py 2015-12-11 04:40:07
> +
> +++ lib/lp/code/browser/tests/test_branchlisting.py 2016-06-28 21:11:44
> +
> @@ -389,7 +387,6 @@
> page = self.get_branch_list_page()
> self.assertThat(page, Not(snaps_matcher))
> with feature_flags():
I think you can drop this context manager block, and also adjust the comment at
the top of this method.
> -set_feature_flag(SNAP_FEATURE_FLAG, u'on')
> page = self.get_branch_list_page()
> if IPerson.providedBy(self.default_target):
> self.assertThat(page, snaps_matcher)
>
> === modified file 'lib/lp/snappy/browser/hassnaps.py'
> --- lib/lp/snappy/browser/hassnaps.py 2016-02-05 14:36:18 +
> +++ lib/lp/snappy/browser/hassnaps.py 2016-06-28 21:11:44 +
> @@ -42,12 +41,9 @@
> # Only enabled if the general snap feature flag is enabled
> # for public contexts and additionally if the snap_private
> # flag is enabled for private contexts.
This comment is no longer quite accurate.
> -if not bool(getFeatureFlag(SNAP_FEATURE_FLAG)):
> -enabled = False
> -else:
> -enabled = (
> -not self.context.private or
> -bool(getFeatureFlag(SNAP_PRIVATE_FEATURE_FLAG)))
> +enabled = (
> +not self.context.private or
> +bool(getFeatureFlag(SNAP_PRIVATE_FEATURE_FLAG)))
>
> text = 'Create snap package'
> return Link('+new-snap', text, enabled=enabled, icon='add')
> @@ -66,9 +62,7 @@
>
> @property
> def show_snap_information(self):
> -return (
> -bool(getFeatureFlag(SNAP_FEATURE_FLAG)) or
> -not self.snaps.is_empty())
> +return not self.snaps.is_empty()
This is only used in one place. I'd be inclined to inline it now (i.e.
tal:condition="not: view/snaps/is_empty").
>
> @property
> def snaps_link(self):
>
> === modified file 'lib/lp/snappy/browser/tests/test_snap.py'
> --- lib/lp/snappy/browser/tests/test_snap.py 2016-05-28 00:21:40 +
> +++ lib/lp/snappy/browser/tests/test_snap.py 2016-06-28 21:11:44 +
> @@ -137,7 +128,6 @@
> branch = self.factory.makeAnyBranch(
> owner=owner, information_type=InformationType.USERDATA)
> with feature_flags():
Is this context manager still needed?
> -set_feature_flag(SNAP_FEATURE_FLAG, u'on')
> with person_logged_in(owner):
> self.assertRaises(
> SnapPrivateFeatureDisabled, create_initialized_view,
>
> === modified file 'lib/lp/snappy/browser/tests/test_snaplisting.py'
> --- lib/lp/snappy/browser/tests/test_snaplisting.py 2016-05-18 12:15:05
> +
> +++ lib/lp/snappy/browser/tests/test_snaplisting.py 2016-06-28 21:11:44
> +
> @@ -39,8 +37,7 @@
> We do things this way rather than by calling self.useFixture because
> opening a URL in a test browser loses the thread-local feature flag.
> """
> -with FeatureFixture({SNAP_FEATURE_FLAG: u"on"}):
> -return self.factory.makeSnap(**kwargs)
> +return self.factory.makeSnap(**kwargs)
The method docstring is no longer accurate. At this point this method is no
longer pulling its weight anyway, so just change self.makeSnap to
self.factory.makeSnap throughout this class and delete the method.
>
> def assertSnapsLink(self, context, link_text, link_has_context=False,
> **kwargs):
>
> === modified file 'lib/lp/snappy/tests/test_snap.py'
> --- lib/lp/snappy/tests/test_snap.py 2016-05-17 12:47:34 +
> +++ lib/lp/snappy/tests/test_snap.py 2016-06-28 21:11:44 +
> @@ -86,18 +83,10 @@
>
> layer = LaunchpadZopelessLayer
>
> -def test_feature_flag_disabled(self):
> -# Without a feature flag, we will not create new Snaps.
> -person = self.factory.makePerson()
> -self.assertRaises(
> -SnapFeatureDisabled, getUtility(ISnapSet).new,
> -person, person, None, None, branch=self.factory.makeAnyBranch())
> -
> def test_private_feature_flag_disabled(self):
> # Without a private feature flag, we will not create new private
> Snaps.
> person = self.factory.makePerson()
> with feature_flags():
Is this context manager still needed?
> -set_feature_flag(SNAP_FEATURE_FLAG, u'on')
> self.assertRaises(
> SnapPrivateFeatureDisabled, getUtility(ISnapSet).new,
> person, person, None, None,
--
https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
___
Mailing list:
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad
* Dropped the snap.allow_new feature flag and fixed lp.snappy tests accordingly. * Synced snappyseries names with production. * Updated snappydistroseries rows to not be full cross product. * Applied changes to current.sql. Thanks! -- https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad has been updated. Commit Message changed to: Enable snappy unconditionally; add snappy sample data for development. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562 -- Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad has been updated. Description changed to: Enable snappy unconditionally; add snappy sample data for development. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562 -- Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad
Review: Needs Fixing Rather than adding it to sampledata, I'd prefer to just drop the snap.allow_new feature flag from the codebase at this point, given that it's been enabled for everyone on all our instances for some time. Please make the snappyseries name columns be in sync with those on production, since that's less confusing. Let's have the sample snappydistroseries rows not be a full cross product. I'd suggest removing (2, 1). After you've dropped the feature flag, can you find out if the lp.snappy tests still pass if you apply this change to current.sql as well, or if it's easy to make them do so? In general I'd prefer to keep the two sets of sampledata in sync if possible. -- https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad.
Commit message:
Add SnappySeries and SnappyDistroSeries sample data for development.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562
Add SnappySeries and SnappyDistroSeries sample data for development.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad.
=== modified file 'database/sampledata/current-dev.sql'
--- database/sampledata/current-dev.sql 2016-05-24 16:52:57 +
+++ database/sampledata/current-dev.sql 2016-06-28 17:59:37 +
@@ -1,6 +1,6 @@
-- Copyright 2010-2016 Canonical Ltd. This software is licensed under the
-- GNU Affero General Public License version 3 (see the file LICENSE).
--- Created using pg_dump (PostgreSQL) 9.3.5
+-- Created using pg_dump (PostgreSQL) 9.3.12
SET check_function_bodies = false;
SET client_encoding = 'UTF8';
@@ -3821,6 +3821,7 @@
INSERT INTO featureflag (scope, priority, flag, value, date_modified) VALUES ('default', 0, 'js.combo_loader.enabled', 'true', '2012-05-18 07:34:39.239649');
INSERT INTO featureflag (scope, priority, flag, value, date_modified) VALUES ('default', 1, 'longpoll.merge_proposals.enabled', 'true', '2012-05-18 07:34:39.239649');
+INSERT INTO featureflag (scope, priority, flag, value, date_modified) VALUES ('default', 0, 'snap.allow_new', 'true', '2016-06-22 15:32:03.502299');
ALTER TABLE featureflag ENABLE TRIGGER ALL;
@@ -9487,6 +9488,15 @@
+ALTER TABLE snappyseries DISABLE TRIGGER ALL;
+
+INSERT INTO snappyseries (id, date_created, registrant, name, display_name, status) VALUES (1, '2016-06-22 16:25:51.474348', 1, 'ubuntu_core_1504', 'Ubuntu Core 15.04', 4);
+INSERT INTO snappyseries (id, date_created, registrant, name, display_name, status) VALUES (2, '2016-06-22 16:25:51.474348', 1, 'ubuntu_core_16', 'Ubuntu Core 16', 4);
+
+
+ALTER TABLE snappyseries ENABLE TRIGGER ALL;
+
+
ALTER TABLE snap DISABLE TRIGGER ALL;
@@ -9524,6 +9534,20 @@
+ALTER TABLE snappydistroseries DISABLE TRIGGER ALL;
+
+INSERT INTO snappydistroseries (snappy_series, distro_series) VALUES (1, 1);
+INSERT INTO snappydistroseries (snappy_series, distro_series) VALUES (1, 3);
+INSERT INTO snappydistroseries (snappy_series, distro_series) VALUES (2, 1);
+INSERT INTO snappydistroseries (snappy_series, distro_series) VALUES (2, 3);
+
+
+ALTER TABLE snappydistroseries ENABLE TRIGGER ALL;
+
+
+
+
+
ALTER TABLE sourcepackageformatselection DISABLE TRIGGER ALL;
INSERT INTO sourcepackageformatselection (id, distroseries, format) VALUES (1, 1, 0);
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
