Simplify

[postmodern]

Made minor simplifications to the internal API for Configuration.

• Use yield instead of Configuration#call. #call should only be defined for objects that represent code, imho.
• Removed Configuration.build in favor of allowing Configuration#initialize to initialize it's instance variables and yielding itself. Yielding from within #initialize and having .new return the new instance is the same behavior as .build.

[solnic]

Thanks, this looks good but before I merge it in (I want to) I need to verify why the hell it was implemented like that in the first place.

That Configuration is some weird stuff. It doesn't look like written by me - why would I make the configuration object mutable, why would I use attr accessors to set its state in the build method. Really really weird. I almost wonder if there wasn't any specific reason for handling it like that.

@elskwid do you maybe remember why it's done like that? I'm completely stumped :)

[blambeau]

@solnic git blame? lol ;-)

[solnic]

I know I wrote it I just don't remember why. It's even bigger lol :)

I actually now see that config is supposed to be mutable. So this behavior has to stay.

On Thu, Apr 17, 2014 at 10:03 AM, Bernard Lambeau notifications@github.com wrote:

@solnic git blame? lol ;-)

Reply to this email directly or view it on GitHub: https://github.com/solnic/virtus/pull/265#issuecomment-40690512

[blambeau]

Aren't tests supposed to tell you why nowadays? I mean BDD and all that jazz.

It's tempting to conclude, therefore, that if tests pass after making config immutable (say), there is no such why and you can move forward. Isn't?

[solnic]

Yes exactly I looked at tests and got my answer. Sorry for the noise ;)

On Thu, Apr 17, 2014 at 10:16 AM, Bernard Lambeau notifications@github.com wrote:

Aren't tests supposed to tell you why nowadays? I mean BDD all that jazz.

It's tempting to conclude, therefore, that if tests pass after making config immutable (say), there is no such why and you can move forward. Isn't?

Reply to this email directly or view it on GitHub: https://github.com/solnic/virtus/pull/265#issuecomment-40691389

[postmodern]

@solnic yeah I got the impression that the global configuration could be modified via Virtus.configuration or Virtus.config (with or without a block) and future Virtus models would inherit the config. Now that I think about it, a global config might be dangerous since it might disable default features needed by other Virtus models.

[solnic]

OK now I remember the story. Goes back to DM1 where configuration was global and associated with type classes. With 1.0 I started the process of moving away from this completely with configuration instances being used by virtus' modules and that's one of the reasons why we have anonymous modules. I suspect it would be already possible to make the config immutable for the generated modules.

[postmodern]

Speaking of which, why is there Virtus.configuration (private API) and Virtus.config (public API)?

[solnic]

It seems redundant and I don't think there's any concrete reason for its existence /cc @elskwid

[elskwid]

@postmodern, @solnic - At the time we didn't have the configurable inclusions, which I know is under discussion, so the ::config method was being used to set the configuration and ::configuration was being used to access the configuration instance. The entire addition of configuration was a retrofit trying to keep the external API. Not ideal. I looked at the diffs and I think ::config was chosen to stay close to the method signatures used when configuring coercers.

Do we need or use it now? If not, I say we get rid of it.

[solnic]

Yeah global config will be deprecated in 1.1 and removed in 2.0.

On Thu, Apr 17, 2014 at 4:29 PM, Don Morrison notifications@github.com wrote:

@postmodern, @solnic - At the time we didn't have the configurable inclusions, which I know is under discussion, so the ::config method was being used to set the configuration and ::configuration was being used to access the configuration instance. The entire addition of configuration was a retrofit trying to keep the external API. Not ideal. I looked at the diffs and I think ::config was chosen to stay close to the method signatures used when configuring coercers.

Do we need or use it now? If not, I say we get rid of it.

Reply to this email directly or view it on GitHub: https://github.com/solnic/virtus/pull/265#issuecomment-40719942