-
Notifications
You must be signed in to change notification settings - Fork 38
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
Tweak rebar.config
(update deps then fixed issue warnings)
#120
Tweak rebar.config
(update deps then fixed issue warnings)
#120
Conversation
rebar.config
rebar.config
(update deps then fixed issue warnings)
CI's failing: I'm up'ing |
5a37fc0
to
943b044
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #120 +/- ##
==========================================
+ Coverage 76.30% 76.40% +0.09%
==========================================
Files 12 12
Lines 764 750 -14
==========================================
- Hits 583 573 -10
+ Misses 181 177 -4 ☔ View full report in Codecov by Sentry. |
I don't see where catch was actually used, since all the clauses derive not from exceptions, but simple function returns
... to most recently expected format
rebar3_lint is 24+, these days, and adding more exceptions to CI just makes it more complex to maintain, I believe
13e7560
to
00df5b4
Compare
Merge conflicts are solved. |
test/elli_tests.erl
Outdated
?assertMatch(invalid_callback, E) | ||
try | ||
elli:start_link([{callback, elli}]) | ||
catch throw:E -> |
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.
Doesn't this change the functionality? Now it is a crash if the exception is anything but throw
, while before catch
handled exit
and error
as well.
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.
This is a test. I thought it should be more specific, since we know what we're testing, but I can go back on that assumption.
Will merge with that one fix. |
Co-authored-by: Tristan Sloughter <[email protected]>
I figure it its a general function in the tests for use anywhere so may as well cover them all. |
Thanks. I'll move to the next one, by rebasing on top of the main branch. |
We update versions of deps and plugins and fix warnings issued from that. It's possible a commit-by-commit review makes it easier to spot how we did stuff.
Further considerations
Most of the changes done previously in #114 where from
elvis_core
's ruleoperator_spaces
, which is disabled in this pull request.