-
Notifications
You must be signed in to change notification settings - Fork 24
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 error with the formatter #16
base: master
Are you sure you want to change the base?
Conversation
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.
👍
if SimpleCov.minimum_coverage[:line]&.positive? | ||
if covered_percent >= SimpleCov.minimum_coverage[:line] |
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.
if SimpleCov.minimum_coverage[:line]&.positive? | |
if covered_percent >= SimpleCov.minimum_coverage[:line] | |
if coverage_minimum&.positive? | |
if covered_percent >= coverage_minimum |
With:
def coverage_minimum
@coverage_minimum ||= begin
minimums = SimpleCov.minimum_coverage
minimums.is_a?(Hash) ? minimums[SimpleCov.primary_coverage] : minimums
end
end
This ensures that we use the minimum corresponding to the primary coverage (which is not necessary :line
)
I implemented your changes, let me know if I need to change anything else ! |
@@ -21,7 +21,7 @@ def format(result) | |||
end | |||
|
|||
private | |||
|
|||
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.
I think you didn't mean to add those spaces here 😜
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.
Could you please remove the empty line?
I'm not affiliated in any way to this project. Was just here because I encountered the same bug you did. Not sure if the owner still maintains this repo. Maybe we can tag them? @MarcGrimme wdyt? |
Using a fork of a fork until this PR gets merged and released: MarcGrimme/simplecov-small-badge#16
We also need some tests (changed to the latest interface of SimpleCov) to cover the behaviour. Could you add that too? |
@@ -21,7 +21,7 @@ def format(result) | |||
end | |||
|
|||
private | |||
|
|||
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.
Could you please remove the empty line?
minimums.is_a?(Hash) ? minimums[SimpleCov.primary_coverage] : minimums | ||
end | ||
end | ||
|
||
def line_coverage_minimum |
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.
Could you also remove the old line_coverage_minimum method. I guess it's not needed any more.
Hi @MarcGrimme I would like to make a suggestion which I hope will unblock this situation: bump the minimum version of simplecov to '0.18'. Currently, this gem supports By excluding versions less than 0.18, there is no longer a need to handle the different types of If that sounds acceptable, I can open a new PR which bumps the simplecov version and includes the new behavior. |
Thanks @joeyates for being constructive. In the mean time I bumped as you proposed the versions used for this gem. We now have a simplecov-small-badge in version 0.2.6 that should support more modern ruby versions (3.2.x). |
There is an error described in the issue #15 .
This implement the described fix and it's now working for the following versions:
SimpleCov 0.21.1
Rspec-rails 4.0.2
Rails 6.1