solnic / virtus

[DISCONTINUED ] Attributes on Steroids for Plain Old Ruby Objects
MIT License
3.77k stars 229 forks source link

Configuration module builder #167

Closed elskwid closed 11 years ago

elskwid commented 11 years ago

Removes the global pollution of class-level configuration while maintaining backwards compatibility with the previous style.

elskwid commented 11 years ago

@solnic - With that last commit the specs I have are running. I believe this is a good starting point for us to discuss if it's operating the way we want it to and if the API is usable.

solnic commented 11 years ago

This is twisted :D and so cool. let's see how it evolves :)

elskwid commented 11 years ago

@solnic, ready for a review at your convenience. Once you've weighed in, I'll make the changes, rebase, split up some commits, and update the PR so we have a proper commit log.

elskwid commented 11 years ago

@solnic: I put in some hours on this today to push forward. I am starting to feel good about the end result. I'm struggling with the best way to unit test ModuleBuilder in the rspec-style you have in this repo. Can we get some time to review and break my brain a little?

elskwid commented 11 years ago

@solnic, I think we've got something here now!

I know we were discussing leaving out the ModuleBuilder specs but I found the coverage had dropped below 100%. I took the opportunity to attempt to get ModuleBuilder#attribute_method and ModuleBuilder#add_included_hook covered. Are these worth making private and skipping the specs? (They feel very internal to me).

Other than that I think we're looking good. I squashed some commits and cleaned up spec style where needed.

Let me know what you see.

solnic commented 11 years ago

@elskwid :heart: :+1:

elskwid commented 11 years ago

:tada: :tada: WOOOO :tada: :tada:

solnic commented 11 years ago

@elskwid btw this probably deserves some info in the README. I plan to extract advanced stuff from README to wiki for 1.0.0 final but we could already have it in the README so I could just copy it later.

elskwid commented 11 years ago

@solnic - You read my mind! I was just looking at the README. I'll get it updated shortly.

phlipper commented 11 years ago

:+1: :sparkles:

elskwid commented 11 years ago

README updated in #174

coveralls commented 10 years ago

Coverage Status

Changes Unknown when pulling ef2204a949a7bc711756dc346f1b612236e86165 on elskwid:module-builder into \ on solnic:master**.