Comments for sam's dm-core  

Awesome! :)

I did not realize the inheritance expectation existed (but see it now in other places). This makes things make more sense, although it means I will need to rewrite some code to be more explicit in many cases. We should probably reflect these expectations in the documentation. For now I will revert the changes.

The intention with DataMapper is that behavior defined outside of an explicit repository block should be inherited by all other repositories. It helps if you try not to think of the “default” repository as being the same as an explicit repository, think of it as the parent to all other repositories, and that they inherit all behavior from it.

I had intended to run against dm-more, but forgot. (they just finished, and did all pass). Thanks for the reminder.

Actually this behavior is wrong. Since the paranoid properties were defined in the default repository, every other respository should inherit the same properties and behavior of the defaul repo. However if you were to define those paranoid properties in a second non-default repository, they should not be shared.

monki, did you also test this with all of sam/dm-more? Changes should only be committed to sam/dm-core if all specs pass for both sam/dm-core and sam/dm-more. I have no reason to believe they will fail, but full testing should be done prior to commits to the stable sam/dm-core branch.

Luis,

Hey Dan,

thnx for fxing this,

Certainly! I’ll make that my next commit.

Luke, would you mind adding a spec for this? Some of the code in ManyToMany associations is rather nasty, and will need some cleanup soon.. I would hate for this behavior to be unknowingly reverted during a refactor session — it is really important to me that DM handle namespaced models.

Could this be done inside a loop to cut down on unecessary boilerplate code? eg:

It’s an issue caused by the constants not being defined until later that this commit was intended to fix, so autoload wouldn’t help.

why not use autoload?

I don’t have Postgres installed. Your change, above, prevented Merb from starting for me:

Would it be possible to just use DataObjects::URI.new here directly?

I’m on it…

I think what sam is trying to say (emphatically ;-) ) is that DM::PropertySet#defaults already returns the properties in a model that are non-lazy, exactly the same as this new DM::Model#eager_properties method does.

pardon? ;-)

Each Repository should get it’s own PropertySet with dups of the properties in the :default PropertySet, so I’m at a loss to explain this. It looks like cruft left-over from another strategy like Properties that can bind on-the-fly to multiple repositories.