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

Don't set session #5

Merged
merged 1 commit into from
May 8, 2018
Merged

Don't set session #5

merged 1 commit into from
May 8, 2018

Conversation

adam12
Copy link
Contributor

@adam12 adam12 commented Jul 21, 2017

I think setting the session should be done by the user inside the yielded block, using the value yielded from the match in #4, if they want that value to be set in the first place.

r.locale do |locale|
  session[:locale] ||= locale
end

If we assume that a session exists, this plugin can't be used where there is no session - ie. a Roda app that performs mailing only.

class Mailer < Roda
  plugin :mailer
  plugin :i18n

  route do |r|
    r.locale do  
      r.mail "/some-path" {}
    end
  end
end

Mailer.sendmail("/en/some-path")  # FAILS! No Rack::Session available

This is likely a breaking change for anybody relying on this functionality.

@kematzy kematzy merged commit 2590196 into kematzy:master May 8, 2018
@kematzy
Copy link
Owner

kematzy commented May 8, 2018

Great catch! Thanks a lot for pull request, and my apologies for not including this PR earlier.

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.

None yet

2 participants