nikhaldi / exception_notification-rake

Sends mail upon failures in Rake tasks
MIT License
39 stars 14 forks source link

Support exception_notification 4.3 #24

Closed TylerRick closed 5 years ago

TylerRick commented 5 years ago

No need to lock to specific patch (or even minor) version if dependencies are following semver since only major versions (like 5.0) may introduce breaking changes to the public API.

lostapathy commented 5 years ago

This seems good to go.

Question for @nikhaldi though - are you open to bumping the required_ruby_version to something like 2.4 or even just 2.3? Dropping support here for a few long-unsupported ruby versions would let us shrink the Travis matrix and remove most of the development dependency logic in the gemspec.

nikhaldi commented 5 years ago

As long as the cost of maintaining support for rubies before 2.3 is quite low I would preserve that.

Do you see specific issues we might run into if we keep support for all rubies >= 2.0? The overhead of the Travis matrix still seems pretty negligible.

I will merge this as is but before a release I'll want to make sure we do some real world integration testing with exception_notification 4.3

lostapathy commented 5 years ago

My big wish would be nice to get the logic based on RUBY_VERSION out of the gemspec. That kind of pattern mostly works for add_development_dependency, but it does not work with add_dependency (it creates broken gems). I dislike conditionals like that at all in a gemspec - eventually somebody gets clever add does an add_dependency and breaks the gem.

Otherwise, it's just a matter of personal preference and some amount of optimism that if libraries quit supporting unsupported runtimes, people will become more apt to upgrade in general, which is good for the ecosystem.