-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[AutoLoad] Autoload Fixes #1904
Merged
Merged
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
f1fb6fe
[Autoload] allows all bits of Grape to be autoloaded
myxoh ae69528
[AutoLoad] creates a compile! method to pre load an API class
myxoh 32fb1d9
[AutoLoad] Addresses rubocop rules, and adds a readme line
myxoh 782509c
[AutoLoad] Adds changes to changelog
myxoh fec0c00
Addresses issues with danger
myxoh e9049bb
Makes sure the check happens inside the Mutex
myxoh 063af30
Syntax on the README
myxoh 35d6473
[AutoLoad] does not take the lock to try to compile is an instance is…
myxoh b46ae0e
[AutoLoad] Tests the new methods compile! and instance_for_rack
myxoh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
Grape.eager_load! | ||
Grape::Http.eager_load! | ||
Grape::Exceptions.eager_load! | ||
Grape::Extensions.eager_load! | ||
Grape::Extensions::ActiveSupport.eager_load! | ||
Grape::Extensions::Hashie.eager_load! | ||
Grape::Middleware.eager_load! | ||
Grape::Middleware::Auth.eager_load! | ||
Grape::Middleware::Versioner.eager_load! | ||
Grape::Util.eager_load! | ||
Grape::ErrorFormatter.eager_load! | ||
Grape::Formatter.eager_load! | ||
Grape::Parser.eager_load! | ||
Grape::DSL.eager_load! | ||
Grape::API.eager_load! | ||
Grape::Presenters.eager_load! | ||
Grape::ServeFile.eager_load! | ||
Rack::Head # AutoLoads the Rack::Head |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be all instances and the method to return an array?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm. I can't remember why this should ever return one of the mounted instances, as you probably want to always be interacting with rack on a predictable instance. I'll have to see what happens if I have this always be the
base_instance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK so this change was initially introduced in: https://github.com/ruby-grape/grape/pull/1893/files
The reason why this is needed is that we used to support - before remounting was introduced - the ability to pre-mount the API you will run in a namespace (and I was maintaing backwards compatibility).
Basically you are allowed to do
And the expectation is that you will be able to get
/namespace/endpoint
I am not sure I want to maintain this behaviour forever, as it doesn't really play nicely with re-mounting an endpoint and I don't find it very sensible (I would find it sensible to
run Namespace
if you want the above behavour, rather than mountingToBeMountedinRack
)Nonetheless, I don't think this is the appropiate PR to introduce this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. We can deprecate this separately.