Code review comment for lp:~mvo/pkgme-devportal/less-silly-contents-file-based-db-backend

On Thu, Aug 16, 2012 at 12:01:31PM -0000, Jonathan Lange wrote:
> Review: Approve
>
> Thanks Michael.
>
> The code is very clear and well-structured.

Happy to hear that!

> I'd draw the lines in slightly different places, but I don't think they are necessary for this to land, or even necessary at all. "Approve with tweaks"

Thank you for your excellent review (as always :)

> Minor typos:
>
> * iter_contents_file has a typo in its header, "wiht" vs "with"
>
> * _download_and_prepare_contents_file_if_needed has a typo in its docstring, "Conents" vs "Contents"
>
> * "interessted" vs "interested" in comments in same function

Fixed in bzr now.

> Some other suggestions:
>
> * _get_lib_to_pkgs_mapping
> * 'cachekey' seems unnecessary. Why not use a tuple as the key?

Indeed, fixed too.

> * Python *almost* has something that does this for you
> (collections.defaultdict). How annoying.

:)
>
 In case you're curious, here's what I think I'd do (or at least would muck around with):
[..]
> Basically, I have a prejudice against look-before-you-leap and another prejudice toward functional programming.

I like your suggestions and will play around with them. I'm not sure I
will do all of them, but I want to apply them and see what the code looks
like then :) Unfortunately I did not get to this yet :/ But I will try
again early next week or after feature freeze (latest).

Thanks,
 Michael