[DBIx-Class-Devel] [dbix-class] Feature/cdbi implicit inflate (#51)

[ungrim97]

15:24 < mrf> ribasushi: pull request sent
15:33 < ribasushi> mrf: several nits
15:34 < ribasushi> I srtongly prefer the tests and implmentation in one commit - allows for proper bisecting
15:34 < ribasushi> you can not assume DateTime is available - grep for "Optional::Dependencies" to see how to use it to properly skip the test when the thing isn't there 
15:35 < ribasushi> please move Date.pm into testlib, and give it a little less generic name
15:36 < ribasushi> the implementation itself - you are subtly changing the if() semantics - previously if there was and inflate/deflate it would do stuff regardless of object type (even on db-stuff) - now this is no longer the case 
15:37 < ribasushi> (I am reading the diff under -w, makes this much more obvious)
15:38 < ribasushi> mrf: other than that - excellent stuff, please see the above comments and resubmit the pr 
15:38 < ribasushi> mrf++
15:38 < mrf> no probs will impliment and chuck back.  
15:38 < mrf> do you want me to squash it down into one commit. 
15:38 < mrf> ?
15:38 < ribasushi> correct
15:39 < ribasushi> mrf: the rationale is: bashing-into-submission history is not valuable enough to make the life of future bisecters/blamers difficult 


---
Reply to this email directly or view it on GitHub:
https://github.com/dbsrgits/dbix-class/pull/51#issuecomment-49445212
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scsys.co.uk/pipermail/dbix-class-devel/attachments/20140718/7c2bd70b/attachment.htm>