Code review comment for lp:~jcsackett/launchpad/registry-errors-649836

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi JC,

Thanks for doing this boring refactoring work. I just have a couple of minor comments below.

-Edwin

>=== modified file 'lib/lp/registry/interfaces/distroseries.py'
>--- lib/lp/registry/interfaces/distroseries.py 2010-09-21 13:05:42 +0000
>+++ lib/lp/registry/interfaces/distroseries.py 2010-09-29 17:42:02 +0000
>@@ -12,7 +12,6 @@
> 'IDistroSeriesEditRestricted',
> 'IDistroSeriesPublic',
> 'IDistroSeriesSet',
>- 'NoSuchDistroSeries',
> ]
>
> from lazr.enum import DBEnumeratedType
>@@ -26,7 +25,6 @@
> operation_returns_collection_of,
> operation_returns_entry,
> rename_parameters_as,
>- webservice_error,
> )
> from lazr.restful.fields import (
> Reference,
>@@ -51,7 +49,6 @@
> from canonical.launchpad.validators.email import email_validator
> from canonical.launchpad.validators.name import name_validator
> from canonical.launchpad.validators.version import sane_version
>-from lp.app.errors import NameLookupFailed
> from lp.app.interfaces.launchpad import IServiceUsage
> from lp.blueprints.interfaces.specificationtarget import ISpecificationGoal
> from lp.bugs.interfaces.bugtarget import (
>@@ -59,6 +56,7 @@
> IHasBugs,
> IHasOfficialBugTags,
> )
>+from lp.registry.errors import NoSuchDistroSeries

It looks like we could use a separate file for special form fields,
since DistroSeriesNameField needs NoSuchDistroSeries, but we lose the
benefit of moving that exception to lp.registry.errors, if we are just
going to import it. Can you open a bug for that?

> from lp.registry.interfaces.milestone import (
> IHasMilestones,
> IMilestone,
>@@ -858,11 +856,5 @@
> """
>
>
>-class NoSuchDistroSeries(NameLookupFailed):
>- """Raised when we try to find a DistroSeries that doesn't exist."""
>- webservice_error(400) #Bad request.
>- _message_prefix = "No such distribution series"
>-
>-
> # Monkey patch for circular import avoidance done in
> # _schema_circular_imports.py
>
>=== modified file 'lib/lp/registry/interfaces/person.py'
>--- lib/lp/registry/interfaces/person.py 2010-09-24 21:10:47 +0000
>+++ lib/lp/registry/interfaces/person.py 2010-09-29 17:42:02 +0000
>@@ -65,9 +61,7 @@
> operation_returns_entry,
> rename_parameters_as,
> REQUEST_USER,
>- webservice_error,
> )
>-from lazr.restful.error import expose
> from lazr.restful.fields import (
> CollectionField,
> Reference,
>@@ -121,6 +115,9 @@
> IHasRequestedReviews,
> )
> from lp.code.interfaces.hasrecipes import IHasRecipes
>+from lp.registry.errors import (
>+ PrivatePersonLinkageError,
>+ )

Can you run utilities/format-imports on all the files you touched
in this branch? This import will fit on a single line.

> from lp.registry.interfaces.gpg import IGPGKey
> from lp.registry.interfaces.irc import IIrcID
> from lp.registry.interfaces.jabber import IJabberID

review: Approve (code)

« Back to merge proposal