Remove reimplementations of String#truncate#2073
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
trichoplax
left a comment
trichoplax
left a comment
There was a problem hiding this comment.
The replacements with truncate and the deprecation of split_words_max_length look good.
I don't feel qualified to approve the email config changes so I've just made this a comment. I've added one question in case it happens to apply.
| # Don't care if the mailer can't send. | ||
| config.action_mailer.raise_delivery_errors = false | ||
| config.action_mailer.delivery_method = :ses | ||
| raise_delivery_errors = ActiveRecord::Type::Boolean.new.cast(ENV['RAISE_DELIVERY_ERRORS']) |
There was a problem hiding this comment.
How sure are we that the ENV variable will be consistently upper or lower case (thinking of other instances in particular - can they manually change it)? Is it worth a defensive conversion to lowercase here?
In the Rails console:
:001> ActiveRecord::Type::Boolean.new.cast('False')
=> true
:002> ActiveRecord::Type::Boolean.new.cast('false')
=> false
:003> ActiveRecord::Type::Boolean.new.cast('FALSE')
=> falseIt appears this is considered correct behaviour and won't be fixed.
There was a problem hiding this comment.
I'm not too fussed about this - it's just for quick dev reconfigurations (I used it to test the error with email sending that we're having at the moment), not long-term production config.
2 participants
Closes #1420.
Removes instances where we've semi-re-implemented
String#truncateand uses that instead. Also deprecatesApplicationHelper#split_words_max_lengthin favour of the former.Unrelated small change: adds additional environment variables for dev environments to control email settings.