Code review comment for lp:~james-w/udd/storm-unicode-fixes

Revision history for this message
James Westby (james-w) wrote :

On Mon, 02 Jul 2012 17:07:23 -0000, Martin Packman <email address hidden> wrote:
> Great, a comment to that effect by the table creation should do.

Done.

> That rule makes sense, but there are currently some exceptions to it.

If you can point them out I'll see about fixing them.

> > Yeah, I should do the cast later when it hits the db, rather than before
> > bzrlib gets it.
>
> Right. Having outstanding_marks deliberately use the db form is a source of some of the pain here.

Yeah, it does make some things a bit odd. Running on postgres will allow
us to delete that code though, because it was a way to avoid sqlite
locking.

> Yup, we just want the import fallback logic in one place then to use the bound name from there. As storm has it, using that everywhere sounds good.

Done.

> > Yep, I assume the test was added after the storm branch was reverted,
> > but didn't cause a test conflict. I can move it in to the earlier branch
> > if you like?
>
> No need, just checking that was the case.

Ok.

Thanks,

James

« Back to merge proposal