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

Make Rails check more specific #447

Merged
merged 1 commit into from Feb 22, 2019

Conversation

paulcsmith
Copy link
Contributor

In one of our non-Rails projects we added ActionMailer. It comes along
with a number of dependencies and modules namespaced under Rails.

The problem is that we don't have a Rails app and therefore Rails.root
is not defined. This causes bullet to blow up when getting Rails.root.

The fix is to be more specific with the check. I also consolidated the
logic to one method since rails? was only ever used to check if Bullet
should call Rails.root. A similar approach is taken in many other libraries and seems to work well: https://github.com/search?q=defined%3F%28Rails.root&type=Code

In one of our non-Rails projects we added ActionMailer. It comes along
with a number of dependencies and modules namespaced under `Rails`.

The problem is that we don't have a Rails app and therefore `Rails.root`
is not defined. This causes bullet to blow up when getting `Rails.root`.

The fix is to be more specific with the check. I also consolidated the
logic to one method since `rails?` was only ever used to check if Bullet
should call `Rails.root`.
@@ -63,6 +63,10 @@ def enable?
!!@enable
end

def app_root
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also moved the logic to one method that can be shared. This way we're not duplicating code across bullet. I hope this was an ok change to make

@smudge
Copy link
Contributor

smudge commented Feb 19, 2019

Found this PR because we have a similar need. We're actually using bullet inside of a Rails engine, and while most of our code is in ./app, the Rails.root that bullet uses is ./spec/dummy. Would be amazing if as part of your PR we could manually override app_root to be specifically not Rails.root, if that makes sense.

Like, during configuration:

Bullet.app_root = Dir.pwd

@flyerhzm flyerhzm merged commit b9ff331 into flyerhzm:master Feb 22, 2019
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

3 participants