I started looking at the scope problems. I made some changes that got all the tests to pass, and then came across Yaroslav''s patch: http://opensource.symetrie.com/trac/better_nested_set/attachment/ticket/7/scoping.diff. The changes Yaroslav and I made are almost identical! I also added aliases for two methods that have been renamed. What are your thoughts on committing these changes? I''m not sure if I have introduced problems for how the scope will be inherited/overridden in descendant classes, but it sounds like scoping was broken anyway. On a related topic, what is your perspective on backwards compatibility? Should we try to play nice with the old nested set for the time being? Krishna -------------- next part -------------- A non-text attachment was scrubbed... Name: scope.diff Type: application/octet-stream Size: 15705 bytes Desc: not available Url : http://rubyforge.org/pipermail/betternestedset-talk/attachments/20061109/781f5c65/attachment-0001.obj
Hi, Le 9 nov. 06, ? 19:33, Krishna Dole a ?crit :> What are your thoughts on committing these changes? I''m not sure if I > have introduced problems for how the scope will be > inherited/overridden in descendant classes, but it sounds like scoping > was broken anyway.Sure. First of all, thks for all the work you put in there. If scope is better fixed, commit. I was wondering wether the patch Yaroslav proposed would break the class root method (I mean, being able to call MyClass.root instead of Myclass.new.root to get the root) where scope is a symbol for a class method. Maybe some tests would help show this behaviour.> On a related topic, what is your perspective on backwards > compatibility? Should we try to play nice with the old nested set for > the time being?DHH clearly stated on rails core list that things like acts_as should one day or another become plugins. So we won''t keep the compatibility for long. My wish would be to be able to label a 0.1 version with tests, preserving all old methods, then for subsequent version we should be free. For instance I want to drop add_child asap. Jean-Christophe Michel -- symetrie.com Better Nested Set for rails: http://opensource.symetrie.com/trac/better_nested_set
Hi,> I was wondering wether the patch Yaroslav proposed would break the > class root method (I mean, being able to call > MyClass.root instead of Myclass.new.root to get the root) where scope > is a symbol for a class method. > Maybe some tests would help show this behaviour.Ah, good point. But I think that it actually doesn''t make sense to use a scope for class methods anyway. If you had three trees, and used :tree_id as your scope, calling MyClass.root wouldn''t make sense-- you haven''t specified which tree_id you want to be the scope. Thoughts? k
Hi, I went ahead and committed the scope changes and aliases I had mentioned before. I hope this is OK-- feel free to revert this if you don''t agree with it. I''ve also just committed fairly comprehensive tests for all methods except the move_to* methods (which need tests the most... ;). All tests now pass (over 200 assertions!). My plan now is to add methods for checking the consistency of trees (I''ve written these for my own projects and just need to port them). After that I will complete the tests for the move_to methods, which will be a lot easier with the error checking methods in place. Hope you''re having a great weekend, Krishna On 11/9/06, Jean-Christophe Michel <jc.michel at symetrie.com> wrote:> Hi, > > Le 9 nov. 06, ? 19:33, Krishna Dole a ?crit : > > What are your thoughts on committing these changes? I''m not sure if I > > have introduced problems for how the scope will be > > inherited/overridden in descendant classes, but it sounds like scoping > > was broken anyway. > > Sure. First of all, thks for all the work you put in there. > If scope is better fixed, commit. > I was wondering wether the patch Yaroslav proposed would break the > class root method (I mean, being able to call > MyClass.root instead of Myclass.new.root to get the root) where scope > is a symbol for a class method. > Maybe some tests would help show this behaviour. > > > On a related topic, what is your perspective on backwards > > compatibility? Should we try to play nice with the old nested set for > > the time being? > > DHH clearly stated on rails core list that things like acts_as should > one day or another become plugins. > So we won''t keep the compatibility for long. > My wish would be to be able to label a 0.1 version with tests, > preserving all old methods, then for subsequent version we should be > free. For instance I want to drop add_child asap. > > Jean-Christophe Michel > -- > symetrie.com > > Better Nested Set for rails: > http://opensource.symetrie.com/trac/better_nested_set > > _______________________________________________ > Betternestedset-talk mailing list > Betternestedset-talk at rubyforge.org > http://rubyforge.org/mailman/listinfo/betternestedset-talk >
Hi, Le 12 nov. 06, ? 00:44, Krishna Dole a ?crit :> I went ahead and committed the scope changes and aliases I had > mentioned before. I hope this is OK-- feel free to revert this if you > don''t agree with it. > > I''ve also just committed fairly comprehensive tests for all methods > except the move_to* methods (which need tests the most... ;). All > tests now pass (over 200 assertions!).Very good job. One doesn''t pass here (postgresql): 1) Failure: test_root(MixinNestedSetTest) [./test/acts_as_nested_set_test.rb:175]: <#<NestedSet:0x15dc054 @attributes {"updated_at"=>nil, "type"=>"NestedSet", "lft"=>nil, "id"=>"3001", "root_id"=>nil, "rgt"=>nil, "pos"=>nil, "parent_id"=>nil, "created_at"=>nil}>> expected but was <#<NestedSet:0x15da31c @attributes {"updated_at"=>nil, "type"=>"NestedSet", "lft"=>nil, "id"=>"3003", "root_id"=>nil, "rgt"=>nil, "pos"=>nil, "parent_id"=>nil, "created_at"=>nil}>>. 29 tests, 209 assertions, 1 failures, 0 errors Another point: shouldn''t we rewrite add_child with current node creation methods ? (even if we drop this method > 0.1)> My plan now is to add methods for checking the consistency of trees > (I''ve written these for my own projects and just need to port them). > After that I will complete the tests for the move_to methods, which > will be a lot easier with the error checking methods in place.Good idea, that''s exactly what I intended to do.> Hope you''re having a great weekend,Fine, just coming back from a concert we gave this aftenoon ;-) Jean-Christophe Michel -- symetrie.com Better Nested Set for rails: http://opensource.symetrie.com/trac/better_nested_set
Hi Jean-Christophe,> Very good job. One doesn''t pass here (postgresql):Ah, I''m glad you are testing on Postgres. I think we should try to run the tests on MySQL, Postgres and SQLite, since those seem to be the most popular. I suppose running on all 3 adapters should be the default rake task. Do you use SQLite? I looked at setting it up, but couldn''t figure out if sqlite or sqlite3 was the version/gem to use.> Another point: shouldn''t we rewrite add_child with current node > creation methods ?Agreed-- I''m planning to do that. Since one of the tests failed on postgres, I guess at this point my plan is: 1) get all 3 databases installed and get the tests to run 2) add and test the error-checking methods 3) rewrite add_child and test the move_to methods> Fine, just coming back from a concert we gave this aftenoon ;-)Ah! What sort of music do you perform? Krishna