This repository was archived by the owner on Jul 19, 2025. It is now read-only.
Add engine_config class #13
Merged
Conversation
This file contains hidden or 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
|
cc @codeclimate/review |
| map[language.downcase] = {} | ||
| end | ||
| elsif languages.is_a?(Hash) | ||
| languages.each_with_object({}) do |language, map| |
There was a problem hiding this comment.
What if we did something like |(key, value), map| here instead of doing the deconstructing in the line below?
There was a problem hiding this comment.
Hah, I did [] last time. Too much Elixir. 😄
|
Good to go? |
| @@ -29,7 +29,7 @@ def run | |||
| end | |||
|
|
|||
| def mass_threshold | |||
There was a problem hiding this comment.
Maybe now is not the time, but it seems like these per-language main classes are a good case for inheritance?
There was a problem hiding this comment.
Yep, I started thinking the same thing and was going to look into it after this PR is good to go.
|
One refactor suggestion, but maybe it should be saved for later. LGTM. |
|
Yeah, I think it's a separate concern so I'll handle that in another PR. |
|
Thanks for the reviews! |
This adds the `EngineConfig` class for normalizing the passed in engine config in a single place instead of using `fetch` all over the codebase with hardcoded paths. This also fixes the path `mass_threshold` looks for when fetching the config.
BlakeWilliams
force-pushed
the
bmw-engine-config-refactor
branch
from
1823f3a to
2b3e74b
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This adds the
EngineConfigclass for normalizing the passed in engineconfig in a single place instead of using
fetchall over the codebasewith hardcoded paths.
This also fixes the path
mass_thresholdlooks for when fetching theconfig.