Skip to content
This repository was archived by the owner on Jul 19, 2025. It is now read-only.

Convert configured mass threshold to integer #20

Merged
merged 1 commit into from
Oct 27, 2015

Conversation

BlakeWilliams
Copy link
Contributor

Currently a configured mass threshold will be returned as a string when
flay expects an integer when it tries to compare mass to mass threshold.

This also renames the assertion since the previous title was incorrect.

@@ -15,7 +15,11 @@ def languages
end

def mass_threshold_for(language)
fetch_language(language).fetch("mass_threshold", nil)
threshold = fetch_language(language).fetch("mass_threshold", nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when this returns nil? Do we default it somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jpignata
Copy link
Contributor

Why isn't the YAML parsing properly casting this value to an integer?

@jpignata
Copy link
Contributor

Do we have an upstream bug where we're incorrectly casting the value from an integer to a string in the YAML -> JSON translation?

@BlakeWilliams
Copy link
Contributor Author

That may be it, by the time it gets to the mass_threshold_for it's a string.

    config:
      languages:
        ruby:
          mass_threshold: 15

Returns "15" when fetching it.

@BlakeWilliams
Copy link
Contributor Author

For reference, when running YAML.load("foo: 15") you get 15 as an integer.

@pbrisbin
Copy link
Contributor

I would have to guess it's this logic that introduced an incorrect cast within the "arbitrary" config node: codeclimate/codeclimate-yaml@5f6323f

@jpignata
Copy link
Contributor

Being defensive here is probably right regardless so 👍 from me on this pull. I'll log a card to look at the upstream bug so it doesn't get lost.

@BlakeWilliams
Copy link
Contributor Author

I also think being defensive is the best approach since it will work even if they pass mass_threshold: "15".

Currently a configured mass threshold will be returned as a string when
flay expects an integer when it tries to compare mass to mass threshold.

This also renames the assertion since the previous title was incorrect.
@gdiggs
Copy link
Contributor

gdiggs commented Oct 27, 2015

👍 as well

@BlakeWilliams BlakeWilliams merged commit 79da837 into master Oct 27, 2015
@BlakeWilliams BlakeWilliams deleted the bmw-threshold-integer branch October 27, 2015 17:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants