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

Use inline conditional instead of Object#try #1485

Closed
wants to merge 4 commits into from

Conversation

daniels
Copy link

@daniels daniels commented Oct 17, 2018

If I'm correct these two lines are the only reason ActiveSupport is a dependency of sinatra-contrib. If so this change adds 10 bytes source code to get rid of 5MB dependencies.

If I'm correct these two lines are the only reason active support is a dependency of sinatra-contrib. If so this change adds 10 bytes source code to get rid of 5MB dependencies.
Non-tested - I only searched for uses on GitHub ...
@mwpastore
Copy link
Member

Also, there's some prior art here: #1208 #1448

@daniels
Copy link
Author

daniels commented Oct 17, 2018

Thanks @mwpastore for your quick reply!

Sorry for storming in without checking for earlier pull requests - saw active support pulled in when updating Sinatra, went straight to the source and found it still there without obvious reason.

Kind of you to provide the feedback even when this pull request is a duplicate. Feel free to close it if #1448 is about to be merged (with or without the change to #respond_to?).

@dentarg
Copy link
Member

dentarg commented Oct 28, 2018

This can be closed now that #1448 has been merged

@namusyaka
Copy link
Member

@daniels Sorry for the late reply. I'll close this since #1448 has been merged

@namusyaka namusyaka closed this Oct 28, 2018
@daniels daniels deleted the patch-1 branch November 20, 2018 14:51
@daniels daniels restored the patch-1 branch November 20, 2018 14:51
@daniels daniels deleted the patch-1 branch November 20, 2018 14:51
@daniels daniels restored the patch-1 branch November 20, 2018 14:51
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.

4 participants