Skip to content

Fix use with keyword arguments for Ruby 3.0#1701

Merged
jkowens merged 1 commit intosinatra:masterfrom
walro:use-with-kwargs
Jun 2, 2021
Merged

Fix use with keyword arguments for Ruby 3.0#1701
jkowens merged 1 commit intosinatra:masterfrom
walro:use-with-kwargs

Conversation

@walro
Copy link
Copy Markdown
Contributor

@walro walro commented May 28, 2021

The added test case would, prior to the fix, fail under Ruby 3.0, with the following error:

MiddlewareTest#test_handles_keyword_arguments_0:
ArgumentError: wrong number of arguments (given 2, expected 1)

The change itself was inspired by rack/rack#1505

@walro
Fix use with keyword arguments for Ruby 3.0
71b9159 
The added test case would, prior to the fix, fail, under Ruby 3.0, with the following error:

```
MiddlewareTest#test_handles_keyword_arguments_0:
ArgumentError: wrong number of arguments (given 2, expected 1)
```

The change itself was inspired by rack/rack#1505
@walro
Copy link
Copy Markdown
Contributor Author

walro commented May 30, 2021

The failure on Ruby-head seems unrelated to the changes here.

@jkowens
Copy link
Copy Markdown
Member

jkowens commented May 30, 2021

@zzak it looks like we have a couple issues with the CI. For some reason native extensions are failing on ruby-head, and we're missing a secret for SLACK_WEBHOOK_URL. I'm not familiar with Github Actions, does it have an "allowed failure" setting? That would be useful for ruby-head which is probably unstable.

@walro
Copy link
Copy Markdown
Contributor Author

walro commented May 31, 2021

and we're missing a secret for SLACK_WEBHOOK_URL

I think that's because of "PRs from forks" are not allowed to access secrets (consider someone doing something like echo $SLACK_WEBHOOK_URL)

I'm not familiar with Github Actions, does it have an "allowed failure" setting?

To my understanding, nothing like Travis CI's allow_failure exists, but continue on error gets you pretty close.

I will submit a PR later today.

@walro
Copy link
Copy Markdown
Contributor Author

walro commented May 31, 2021 •
edited
Loading

I will submit a PR later today.

See #1703

Edit: That PR did not solve "fails on ruby-head", but it at least disables fail-fast which is a bit of an improvement.

@jkowens jkowens merged commit 0a50b56 into sinatra:master Jun 2, 2021
@jkowens
Copy link
Copy Markdown
Member

jkowens commented Jun 2, 2021

Thanks for your contributions!

@tonytonyjan tonytonyjan mentioned this pull request Jun 3, 2021
Closed
@epergo epergo mentioned this pull request Sep 7, 2021
Closed
@dentarg dentarg mentioned this pull request Sep 22, 2021
Closed
@dentarg dentarg mentioned this pull request Jan 3, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@walro @jkowens