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

Rename Configuration#modules into #prepare #57

Closed
jodosha opened this issue Dec 5, 2014 · 3 comments · Fixed by #58
Closed

Rename Configuration#modules into #prepare #57

jodosha opened this issue Dec 5, 2014 · 3 comments · Fixed by #58

Comments

@jodosha
Copy link
Member

jodosha commented Dec 5, 2014

Please remember to update the README too.

@runlevel5
Copy link
Member

@jodosha can you please explain the motive behind this decision?

@jodosha
Copy link
Member Author

jodosha commented Dec 5, 2014

@joneslee85 IMO that initial name was wrong: we don't define the modules to include, but we pass a code block. That block is evaluated when Lotus::Action gets included. So we are preparing the action.

In code terms, if it was something like the following code, it could've had a sense to name it #prepare.

configure do
  modules Lotus::Action::Session, Lotus::Action::Cookies
end

But with the current implementation we can do something more than listing modules:

configure do
  prepare do
    include Lotus::Action::Sessions
    use SomeMiddleware
    before :do_something
  end
end

What do you think?

@runlevel5
Copy link
Member

@jodosha i see. I am cool with the idea of passing a code block but I am wondering how you extend/override?

runlevel5 pushed a commit to runlevel5/controller that referenced this issue Dec 5, 2014
runlevel5 pushed a commit to runlevel5/controller that referenced this issue Dec 5, 2014
runlevel5 pushed a commit to runlevel5/controller that referenced this issue Dec 5, 2014
runlevel5 pushed a commit to runlevel5/controller that referenced this issue Dec 5, 2014
runlevel5 pushed a commit to runlevel5/controller that referenced this issue Dec 5, 2014
runlevel5 pushed a commit to runlevel5/controller that referenced this issue Dec 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants