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

Allow session configuation #57

Merged
merged 2 commits into from Dec 1, 2014
Merged

Allow session configuation #57

merged 2 commits into from Dec 1, 2014

Conversation

michalmuskala
Copy link
Contributor

  • add #sessions Lotus::Configuration, used to enable and configure sessions
  • add Lotus::Config::Sessions responsible for sessions configuration and
    sessions middleware construction
  • move assets middleware construction to Lotus::Config::Assets to be
    consistient with sessions
  • include Lotus::Action::Session to controllers if sessions are enabled
  • defer loading default middleware stack (assets and sessions) to load time,
    and not middleware stack initialization.
  • change Lotus::Config::Middleware tests to account for the
    configuration loading that happens at application initialization, test
    for disabled assets was false positive in previous setup

Closes #2

* add #sessions Lotus::Configuration, used to enable and configure sessions
* add Lotus::Config::Sessions responsible for sessions configuration and
  sessions middleware construction
* move assets middleware construction to Lotus::Config::Assets to be
  consistient with sessions
* include Lotus::Action::Session to controllers if sessions are enabled
* defer loading default middleware stack (assets and sessions) to load time,
  and not middleware stack initialization.
* change Lotus::Config::Middleware tests to account for the
  configuration loading that happens at application initialization, test
  for disabled assets was false positive in previous setup

Closes #2
@jodosha
Copy link
Member

jodosha commented Jul 20, 2014

@michalmuskala Thanks for this PR. I'll dig into the code at the beginning of the next week, as I'm traveling right now. Let's discuss later on.

@@ -27,6 +27,10 @@ def enabled?
!@path.nil?
end

def middleware
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to keep the Rack concern out of this. This object abstracts some file system knowledge, better to keep it simple. Middleware should be the right place for this.

* Move assets and sessions middleware configuration to
  Config::Middleware
* Move default session options to Config::Sessions
@jodosha jodosha merged commit 842c6ef into hanami:master Dec 1, 2014
jodosha added a commit that referenced this pull request Dec 1, 2014
@jodosha
Copy link
Member

jodosha commented Dec 1, 2014

@michalmuskala I've finally merged this. It will be part of the upcoming 0.2.0. Thank you! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to configure sessions
2 participants