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

Feature request - add a cop to detect missing :environment dependencies in rake tasks #63

Closed
scottjacobsen opened this issue Jan 8, 2019 · 5 comments
Labels
enhancement New feature or request feature request Request for new functionality

Comments

@scottjacobsen
Copy link

I'm always frustrated when I commit a rake task to my rails project and I forget the :environment dependency. For example:

task :fix_something do # should be `task fix_something: :environment do`
   User.where(...).find_each { |u| # fix the user }
end

This runs just fine in my dev environment, probably because spring is running and just forks off a process where the whole rails env is loaded. So I push to production. Later on production

rake fix_something
rake aborted!
NameError: uninitialized constant User

:rage4:

Describe the solution you'd like

I would like a rails cop that warns if you write a rake task without an environment dependency. It is rarely what you want. It is also easy to miss in code reviews.

@stale
Copy link

stale bot commented May 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@tejasbubane
Copy link
Contributor

Rails-related functionality has been moved to a standalone library (gem) named rubocop-rails. Please, migrate this issue to its issue tracker https://github.com/rubocop-hq/rubocop-rails/issues/

@koic koic transferred this issue from rubocop/rubocop May 31, 2019
@andyw8
Copy link
Contributor

andyw8 commented Jun 1, 2019

I think this could be helpful, but there are valid situations where a task doesn't depend on Rails, e.g.:

  task :clear_pending_uploads do
    run "rm -f uploads/pending"
  end

So this might result in a lot of false positives.

Checking real-world-rails:

real-world-rails master % ag "task \:.*environment" . -G rake | wc -l
    1054
real-world-rails master % ag "task \:" . -G rake | wc -l
    1957

So around half of all entries don't have a direct :environment dependency.

@pocke
Copy link
Contributor

pocke commented Nov 27, 2019

Sorry I overlooked this issue, but I implemented a cop that has the same behavior with this issue.#130

I think this could be helpful, but there are valid situations where a task doesn't depend on Rails, e.g.:

Interesting. When I investigated with our private application, I found most task definitions have :environment dependency. So I guessed it is ok.

But if people get many false positives, I guess we can improve the cop. Maybe the cop looks the content of rake task, or disable it by default.

I'll close this issue because it is a feature request and it's done. But feel free to open a new issue if you have any problems. Thanks!

@pocke pocke closed this as completed Nov 27, 2019
@pocke pocke added enhancement New feature or request feature request Request for new functionality labels Nov 27, 2019
@rpbaptist
Copy link

Well, looks like this was implemented without it being fully featured. Sometimes you don't want to load in the entire environment for a simple task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature request Request for new functionality
Projects
None yet
Development

No branches or pull requests

5 participants