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

Replace with new router (discussion) #1800

Merged
merged 54 commits into from
Nov 6, 2014
Merged

Replace with new router (discussion) #1800

merged 54 commits into from
Nov 6, 2014

Conversation

ujifgc
Copy link
Member

@ujifgc ujifgc commented Oct 29, 2014

Oh, such a nice number, #1800!

namusyaka and others added 30 commits September 16, 2013 07:08
…up-exceptions

Bubble up router exceptions
- the `params` variable is converted into a HashWithIndifferentAccess
  object
- fixes #1405

Conflicts:

	padrino-core/lib/padrino-core/application/routing.rb
Conflicts:

	padrino-core/lib/padrino-core/mounter.rb
Change Mustermann::Rails to Mustermann::Sinatra.
Conflicts:
	padrino-core/lib/padrino-core/application/routing.rb
	padrino-core/lib/padrino-core/support_lite.rb
	padrino-core/test/test_routing.rb
Conflicts:
	Gemfile
	padrino-core/lib/padrino-core/application/routing.rb
	padrino-core/padrino-core.gemspec
	padrino-core/test/test_routing.rb
Conflicts:
	padrino-core/lib/padrino-core/application/routing.rb
Conflicts:
	padrino-core/lib/padrino-core/application/routing.rb
Conflicts:
	padrino-core/lib/padrino-core/application/routing.rb
namusyaka and others added 3 commits September 25, 2014 01:40
Conflicts:
	padrino-core/lib/padrino-core/application/routing.rb
	padrino-core/lib/padrino-core/ext/http_router.rb
	padrino-core/lib/padrino-core/loader.rb
	padrino-core/test/test_application.rb
@@ -29,7 +29,11 @@ Gem::Specification.new do |s|
else
s.add_dependency("sinatra", "~> 1.4.2")
end
s.add_dependency("http_router", "~> 0.11.0")
if RUBY_VERSION < '2.0.0'
Copy link
Member Author

Choose a reason for hiding this comment

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

This condition won't work in gemspec. It's evaluated on the developer's box, not on user's one.

Choose a reason for hiding this comment

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

What about giving a warning on install about this or just appending it to the Gemfile?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, thanks. If I understand things correctly, we should use mustermann19 because the official mustermann doesn't support for rubinius and jruby yet.

@namusyaka namusyaka self-assigned this Oct 29, 2014
@ujifgc
Copy link
Member Author

ujifgc commented Oct 29, 2014

For this test.
Mustermann19's default encoding is ASCII on the ruby1.9 environment.
Any ideas?

It's not mustermann19, it's Rack's encoding. And ruby2 seems to silently re-encode the request path_info on demand. Looks fixed after 54d350c

@namusyaka
Copy link
Contributor

Great, I've misunderstood. Thanks!

@ujifgc ujifgc added this to the 0.13.0 milestone Oct 30, 2014
@ujifgc
Copy link
Member Author

ujifgc commented Nov 6, 2014

Should we just merge it to master and bump the version to 0.13.0.rc1? This PR is beginning to look like a mess.

@namusyaka
Copy link
Contributor

Yes. We can merge this branch if there is no problem.

@dariocravero
Copy link

👍
On 6 Nov 2014 07:03, "namusyaka" notifications@github.com wrote:

Yes. We can merge this branch if there is no problem.


Reply to this email directly or view it on GitHub
#1800 (comment)
.

ujifgc added a commit that referenced this pull request Nov 6, 2014
@ujifgc ujifgc merged commit 2f079c3 into master Nov 6, 2014
@ujifgc ujifgc deleted the replace-with-new-router branch November 6, 2014 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants