search

datamapper / dm-validations

Library for performing validations on DM models and pure Ruby object
http://datamapper.org/
MIT License
50 stars 43 forks source link

Feature/extract responsibilities #27

Closed emmanuel closed 13 years ago

emmanuel commented 13 years ago

This is the third of three pull requests, and contains the most significant changes. This is dependent on the changes in the first pull request, but not on the second.

xaviershay commented 13 years ago

Thanks for these - I've commented on individual commits where I had questions. Overall, can you give me some context about these changes? Are you refactoring so you can achieve something you couldn't prior, or are you just giving it a spring clean?

emmanuel commented 13 years ago

Heh, oops; pardon the context-free refactor. I guess 'spring cleaning' about sums it up. I went trolling around through some of the DM libs when I started working on my current project (which uses DM).

In general I found the source in dm libs quite readable and good, but I also found some things that looked like they could be streamlined, so I earmarked them for clean up when I got the time (I also have pull requests open on dm-types and dm-core, so far).

Some of the changes I've proposed are stylistic and some are subjective design choices that I misread. For example I subsumed dm-timestamps into a couple of new classes in dm-types, but that isn't aligned with the responsibilities of Property, so that patch was rejected.

Finally, I do also have a little thought about spring cleaning dm-validations so it can more easily be integrated with virtus. But that's an idea in the abstract; these changes certainly do not relate to any integration plan.

xaviershay commented 13 years ago

Excellent. I don't have any objections to your changes, so will pull them in shortly.

xaviershay commented 13 years ago

Closing, covered by a newer pull request.