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.

Sign up for GitHub

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

Jump to bottom

Fix usage of inherited Sinatra::Base classes keyword arguments #1670

Merged
merged 10 commits into from Jan 7, 2021
+21 −3
Merged

Fix usage of inherited Sinatra::Base classes keyword arguments #1670

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
64347b0
Fix usage of inherited Sinatra::Base classes keyword arguments
duduribeiro Dec 17, 2020
06e049c
Include tests
duduribeiro Dec 17, 2020
561ec54
Import rvm Key on travis pipeline
duduribeiro Dec 17, 2020
be56991
Trying to use ruby-head instead of ruby-3.0.0-preview1 to avoid errors
duduribeiro Dec 17, 2020
3c80f4f
Returning ruby-3.0.0 to test it on xenial linux
duduribeiro Dec 18, 2020
a5ed7fb
Upgrade to Xenial
duduribeiro Dec 18, 2020
605d7bf
Merge branch 'master' into fix_positional_arguments_in_ruby_3
duduribeiro Dec 29, 2020
b979ca6
Apply suggestions from code review
duduribeiro Dec 29, 2020
2760659
Change initialize method to accept kwargs
duduribeiro Dec 30, 2020
77e6872
Merge branch 'master' into fix_positional_arguments_in_ruby_3
duduribeiro Dec 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion .travis.yml
View file
Open in desktop
@@ -1,7 +1,7 @@
---
language: ruby

dist: trusty
dist: xenial
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On trusty rvm is failing to install ruby 3.0.0-preview1, but upgrading to xenial makes the jruby build fails.

Copy link
Member

Choose a reason for hiding this comment

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

I think we should try to get the test suite running on GitHub Actions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dentarg wanna merge this and work on GH actions after? I can work on this in a new PR if I get access on it.

Also, this will be importante because:
https://blog.travis-ci.com/2020-11-02-travis-ci-new-billing

Building on a public repositories only
We love our OSS teams who choose to build and test using TravisCI and we fully want to support that community. However, in recent months we have encountered significant abuse of the intention of this offering (increased activity of cryptocurrency miners, TOR nodes operators etc.). Abusers have been tying up our build queues and causing performance reductions for everyone. In order to bring the rules back to fair playing grounds, we are implementing some changes for our public build repositories.
For those of you who have been building on public repositories (on travis-ci.com, with no paid subscription), we will upgrade you to our trial (free) plan with a 10K credit allotment (which allows around 1000 minutes in a Linux environment).

Copy link
Member

Choose a reason for hiding this comment

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

@duduribeiro I'm not maintainer or collaborator, just a fellow contributor :-) It is @namusyaka or @jkowens that can merge PRs

Copy link
Member

Choose a reason for hiding this comment

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

I think Actions should work in your fork of Sinatra, so you can work on it right now if you want

Copy link
Member

Choose a reason for hiding this comment

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

Switching to Github Actions would be 💯


before_install:
- gem install bundler
Expand All @@ -20,6 +20,7 @@ rvm:
- 2.5.8
- 2.6.6
- 2.7.1
- 3.0.0-preview1
- jruby-9.2.12.0

script: ./.travis.sh
Expand All @@ -28,6 +29,7 @@ matrix:
allow_failures:
- rvm: 2.6.6
- rvm: jruby-9.2.12.0
- rvm: 3.0.0-preview1

notifications:
slack:
Expand Down
4 changes: 2 additions & 2 deletions lib/sinatra/base.rb
View file
Open in desktop
Expand Up @@ -1522,8 +1522,8 @@ def prototype
# Create a new instance of the class fronted by its middleware
# pipeline. The object is guaranteed to respond to #call but may not be
# an instance of the class new was called on.
def new(*args, &bk)
instance = new!(*args, &bk)
def new(*args, **kwargs, &bk)
instance = kwargs.length > 0 ? new!(**kwargs, &bk) : new!(*args, &bk)
jkowens marked this conversation as resolved.
Show resolved Hide resolved
Wrapper.new(build(instance).to_app, instance)
end

Expand Down
18 changes: 18 additions & 0 deletions test/base_test.rb
View file
Open in desktop
Expand Up @@ -6,6 +6,14 @@ class TestApp < Sinatra::Base
get('/') { 'Hello World' }
end

class TestKeywordArgumentInitializerApp < Sinatra::Base
def initialize(argument:)
@argument = argument
end

get('/') { "Hello World with Keyword Arguments: #{@argument}" }
end

it 'include Rack::Utils' do
assert TestApp.included_modules.include?(Rack::Utils)
end
Expand Down Expand Up @@ -48,6 +56,16 @@ class TestApp < Sinatra::Base
TestApp.configure { context = self }
assert_equal self, context
end

it "allows constructor to receive keyword arguments" do
app = TestKeywordArgumentInitializerApp.new(argument: "some argument")
request = Rack::MockRequest.new(app)

response = request.get('/')

assert response.ok?
assert_equal 'Hello World with Keyword Arguments: some argument', response.body
end
end

describe "Sinatra::Base#new" do
Expand Down