textmate / ruby.tmbundle

TextMate support for Ruby
178 stars 91 forks source link

Rename assert_raise to assert_raises #127

Closed hovsater closed 4 years ago

hovsater commented 5 years ago

For Minitest compatibility we rename assert_raise to assert_raises. Since the TestUnit gem have an alias for it, it will both work in both cases.

infininight commented 5 years ago

Do you know what the adoption of both of these test libraries is? Even though one works for both I worry that it being the alias might confuse some people since it has been this way for so long?

hovsater commented 5 years ago

I'd say minitest is the most common used of the two. It's also what Rails use under the hood.

If we're not interested in compatibility between the two, I'd say making Minitest specific changes make the most sense.

noniq commented 4 years ago

I’m in favour of this change. Since it is backwards-compatible anyway, I don’t see any negative impact.

On a more general note, I’m aware that there’s a delicate balance to keep between “confusing long time users through changes” and “making changes to keep up with evolving and changing eco-systems”. However, my general impression is that a lot of bundles are gradually getting more and more out of date (especially thinking of the Ruby on Rails bundle here). That’s why I think changes going in this direction should rather be accepted 😃

sorbits commented 4 years ago

[…] That’s why I think changes going in this direction should rather be accepted

I’m definitely in favor of evolving the bundles together with the ecosystems that they support. Users can always checkout older versions, should there be some change that “breaks” compatibility but is deemed necessary for “modern” workflows; such cases though I think would be rare. If it’s e.g. about updating Ruby on Rails, we can tag the bundle before making major changes, just to have a proper record of when support for version x.y was “officially” dropped.

noniq commented 4 years ago

Thanks for the clarification, @sorbits. And thanks for the pull request, @KevinSjoberg, it just got merged!