Merge lp:~jcsackett/launchpad/convert-sql-627631-storm into lp:launchpad/db-devel
Status: | Merged |
---|---|
Approved by: | j.c.sackett |
Approved revision: | no longer in the source branch. |
Merged at revision: | 10000 |
Proposed branch: | lp:~jcsackett/launchpad/convert-sql-627631-storm |
Merge into: | lp:launchpad/db-devel |
Prerequisite: | lp:~jcsackett/launchpad/convert-sql-627631 |
Diff against target: |
233 lines (+62/-25) 7 files modified
lib/canonical/launchpad/database/launchpadstatistic.py (+7/-5) lib/lp/registry/model/product.py (+5/-1) lib/lp/registry/model/projectgroup.py (+18/-13) lib/lp/translations/model/translationsperson.py (+7/-2) lib/lp/translations/scripts/tests/test_translations_to_branch.py (+2/-1) lib/lp/translations/scripts/translations_to_branch.py (+6/-3) lib/lp/translations/stories/standalone/xx-products-with-translations.txt (+17/-0) |
To merge this branch: | bzr merge lp:~jcsackett/launchpad/convert-sql-627631-storm |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Edwin Grubbs (community) | code | Approve | |
Benji York (community) | code | Approve | |
Review via email: mp+41011@code.launchpad.net |
Commit message
[r=benji,
Description of the change
Summary
=======
There's an ongoing effort to phase out the official_* booleans which indicate how a product/
This branch rectifies that by cleaning up those sections of code to reference the EnumCol, so that we can (if we choose to) drop the official_* code completely.
This branch was split out separately from the one it depends on b/c it seemed that storm queries needed special handling, and were related to some other issues in the code base.
Proposed fix
============
Replace locations making Storm queries and update them to use the EnumCol and ServiceUsage DBItem.
Preimplementation talk
=======
Spoke with Curtis Hovey.
Implementation details
=======
As in proposed. Additionally, as low hanging fruit a query was converted to storm to fix a related issue in projectgroups.
Tests
=====
Because of the number of things that end up effected, most modules need testing. Testing registry alone is a reasonable proxy, but not ideal.
bin/test -m lp.registry.tests
OR
bin/test -m lp.registry
bin/test -m lp.translations
bin/test -m lp.answers
Demo & QA
=========
Operation of the various main pages (e.g. +translations, +questions) for a product or distribution shouldn't fail in anyway or appear different; b/c of prior work this change should be invisible.
Lint
====
make lint output:
= Launchpad lint =
2
3 Checking for conflicts and issues in changed files.
4
5 Linting changed files:
6 lib/canonical/
7 lib/lp/
8 lib/lp/
9 lib/lp/
10 lib/lp/
11 lib/lp/
12 lib/lp/
13 lib/lp/
14 lib/lp/
15
This branch looks good.
Replacing hard-coded SQL with Storm expressions is a nice bonus.
I'm curious why _translations_usage has a leading underscore, but that
wasn't introduced in this branch, so not really relevant.
Most people include the lint output as you do, but personally I prefer
to include the output of lint /before/ I fix the things it notes. That
way the reviewer can know which parts of the diff were lint and which
were integral to the task at hand. Not a big deal, and an opinion that
may not be shared by others; just something to think about.