Hi Gaetan, > Other, minor complaints include (in arbitrary order and without any > "gentle wrapping" -- I hope you don't take them badly:
Well, I had hoped for a clean run getting this committed, but all your comments are fair points :-) > could see your code only handle columns and *list* > properties/relationships, and thus cannot be used to set a ManyToOne > relationship. Bum... that may be a little tricky to fix; I'll have a go. > - Is the "dbdata.remove(delobj)" line useful? (it seem useless to me > but I could be wrong) Tests fail without it, although I think an extra session.flush() would make them pass. I reckon leave it in. > Also, I'd personally prefer if the deep_set was a method on the entity My thinking for keeping it separate was that it can work on non-Elixir classes as it is, which may be important in the future when you can have relationships between Elixir and raw-SA classes. On balance, think I'll go with your suggestion; we can always backtrack in the future. > (probably named "from_dict") and the "to_json" method was renamed to > "to_dict" as the output is not really json and I like to have Yes, those are better names. In fact, shall we leave set() as it is, so simple sets remain performant? > Seem pretty clean to me. What don't you like about it? The deep parameter seems a bit weird. Anyway, screw it, I'll leave it in. Stay tuned for the next patch! Paul --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "SQLElixir" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/sqlelixir?hl=en -~----------~----~----~----~------~----~------~--~---
