Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Anima::Defaults #12

Closed
wants to merge 1 commit into from
Closed

Conversation

plexus
Copy link

@plexus plexus commented Oct 22, 2014

Add a way to specify defaults for certain attributes.

Probably not up to your standards, but the basic idea is there. Had some issues running mutant, this will need some more coverage.

Add a way to specify defaults for certain attributes.

def included(descendant)
descendant.instance_exec(@defaults) do |defaults|
define_singleton_method(:anima_defaults) { defaults }
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a getter for the anima module. AnimaInfectedClass.anima returning an Anima instance.

This code adds an other getter AnimaInfectedClass.anima_defaults, this is kind of aname duplication with the .anima method above, we could avoid in case we refactor the code to attach itself to the Anima module instance. Defaults would be accessed via AnimaInfectedClass.anima.defaults. Resulting in minimum namespace cluttering.

@mbj
Copy link
Owner

mbj commented Oct 26, 2014

@plexus I just finished a quick code only pass, and had some code flags that we should be able to resolve fairly quickly. As I like the overall idea of backing in default arguments I can offer to do the required fixes by myself or just you deal with the comments of mine. Just tell me how you'd like to proceed.

@mbj
Copy link
Owner

mbj commented Nov 9, 2014

@plexus Did my comments/review scared you to death ;) ?

You maybe just lost my last comment asking if I should take over.

@plexus
Copy link
Author

plexus commented Nov 9, 2014

@mbj yeah this just ended up on the backburner somehow. Wouldn't mind revisiting, but not high prio at the moment. So if you feel like taking over then sure go ahead.

@mbj
Copy link
Owner

mbj commented Nov 9, 2014

@plexus I'll revisit it the next time I stumble over:

class Foo
  include Anima.new(:a, :b)
  DEFAULTS = IceNine.deep_freeze(
    a: 1,
    b: 2
  )
  def self.new(attributes)
    super(DEFAULTS.merge(attributes))
  end
end

in my code, and seeing myself writing specs for it.

@mbj
Copy link
Owner

mbj commented Jan 11, 2015

Closing for no activity. @plexus I'm happy to accept this PR when the issues I flagged are fixed. I do not realistically have the time to do this so I'm closing for now to keep the noise in my trackers down.

@mbj mbj closed this Jan 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants