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
[Fix #10842] Make server mode aware of CacheRootDirectory
and two env vars
#10852
[Fix #10842] Make server mode aware of CacheRootDirectory
and two env vars
#10852
Conversation
e2aeaf4
to
66b8845
Compare
CacheRootDirectory
and two env varsCacheRootDirectory
and two env vars
66b8845
to
e3c05aa
Compare
lib/rubocop/config_path_finder.rb
Outdated
module RuboCop | ||
# This class has methods related to finding configuration path. | ||
# @api private | ||
class ConfigPathFinder |
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.
Perhaps just ConfigFinder
?
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.
Thanks for the suggestion! I've update this class name.
lib/rubocop/cache_root.rb
Outdated
module RuboCop | ||
# This class represents the cache root of the caching RuboCop runs. | ||
# @api private | ||
class CacheRoot |
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.
CacheRoot
seems a bit too specific to me to warrant a class - maybe this should be CacheConfig
or something like this?
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.
CacheConfig
looks good! Also changed the class method name from CacheRoot.dir
to CacheConfig.root_dir
.
…d two env vars Fixes rubocop#10842 and follow up rubocop#10849. This PR makes server mode aware of `CacheRootDirectory` config option value, `RUBOCOP_CACHE_ROOT`, and `XDG_CACHE_HOME` environment variables. `RuboCop::ConfigLoader` and `RuboCop::ResultCache` classes to get these values has many dependencies. So, it is not suitable for use in server mode where speed is required. This solution is to extract `ConfigPathFinder` class from `ConfigLoader` class and `CacheRoot` class from `ResultCache` class. This allows server mode to depend on lightweight `ConfigPathFinder` and `CacheRoot` classes.
e3c05aa
to
651ba56
Compare
Thanks! |
…d two env vars (rubocop#10852) Fixes rubocop#10842 and follow up rubocop#10849. This PR makes server mode aware of `CacheRootDirectory` config option value, `RUBOCOP_CACHE_ROOT`, and `XDG_CACHE_HOME` environment variables. `RuboCop::ConfigLoader` and `RuboCop::ResultCache` classes to get these values has many dependencies. So, it is not suitable for use in server mode where speed is required. This solution is to extract `ConfigFinder` class from `ConfigLoader` class and `CacheConfig` class from `ResultCache` class. This allows server mode to depend on lightweight `ConfigFinder` and `CacheConfig` classes.
Fixes #10842 and follow up #10849.
This PR makes server mode aware of
CacheRootDirectory
config option value,RUBOCOP_CACHE_ROOT
, andXDG_CACHE_HOME
environment variables.RuboCop::ConfigLoader
andRuboCop::ResultCache
classes to get these values has many dependencies. So, it is not suitable for use in server mode where speed is required.This solution is to extract
ConfigPathFinder
class fromConfigLoader
class andCacheRoot
class fromResultCache
class. This allows server mode to depend on lightweightConfigPathFinder
andCacheRoot
classes.Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.