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

Splitting sinatra/base.rb #1543

Closed
wants to merge 1 commit into from
Closed

Conversation

halan
Copy link

@halan halan commented Jun 6, 2019

This PR just splits sinatra/base.rb into files. The intent here is to do the first step of a refactoring. The next steps can be refactoring each piece separately.

Related with #1531

@halan halan force-pushed the splitting-base branch 2 times, most recently from 7200c94 to 0271b32 Compare June 6, 2019 01:11
@namusyaka namusyaka added this to the v3.0.0 milestone Jun 23, 2019
@halan
Copy link
Author

halan commented Jun 23, 2019

:shipit:

@adbatista adbatista mentioned this pull request Jun 24, 2019
@halan
Copy link
Author

halan commented Aug 21, 2019

rebased. ❤️ Let's rock!

Copy link
Contributor

@vbrazo vbrazo left a comment

Choose a reason for hiding this comment

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

@halan What do you think about adding these error objects in a errors.rb and centralize them there?

module Sinatra
  module Errors
    # Raised when the server was unable to process the request sent by the client due to invalid syntax
    class BadRequest < TypeError

    # Raised when a URL is no longer accessible
    class NotFound < NameError
  end
end

@halan
Copy link
Author

halan commented Aug 21, 2019

Sounds good. I'll take a look.

@janko
Copy link

janko commented Aug 22, 2019

Top-level constants living in lib/sinatra/base/*.rb should probably live in lib/sinatra/*.rb instead.

@halan
Copy link
Author

halan commented Aug 22, 2019

I agree.

@halan
Copy link
Author

halan commented Oct 23, 2020

I did rebase with master and accepted all good suggestions here! Thank you guys.

@jubishop
Copy link

Will this ever get accepted?

@jkowens jkowens changed the base branch from master to unstable November 2, 2021 02:45
@jkowens jkowens deleted the branch sinatra:unstable February 2, 2022 21:09
@jkowens jkowens closed this Feb 2, 2022
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

8 participants