Code review comment for lp:~jml/pkgme-devportal/use-description

Revision history for this message
Jonathan Lange (jml) wrote :

On Mon, Dec 19, 2011 at 7:51 PM, James Westby <email address hidden> wrote:
> Review: Needs Fixing
>
> Hi,
>
> Here's the things we spoke about on the phone:
>
> 17      + with open(METADATA_FILE) as f:
> 18      + metadata = json.load(f)
> 19      + print metadata[DESCRIPTION]
>
> There should be a helper method to do that in devportalbinary.binary as it's repeated a lot.
>
> Also, this should be the same for the pdf backend, which you added a card for.
>

I've added the helper method and made both backends use it. The card
can be for reducing the amount of shared code between the backends
now.

> The description needs specific formatting, but that should be done in pkgme.
>

Check.

> 135     -from sqlite3 import IntegrityError
> 136     +from pysqlite2.dbapi2 import IntegrityError
>
> That doesn't work on my oneiric install, so I think we'll need at least try_import or similar.
> However, sqlite3 is shipped in the python2.7 package here, so I'm not sure why you don't
> have it on precise.
>

It's not an import problem on precise. I have sqlite3. The problem is
that the IntegrityError raised by Storm is a different integrity error
on precise. Will find some sort of compatibility work-around.

> I think that only the last is a blocker.
>

Thanks,
jml

« Back to merge proposal