Make #to_fs the default replacement for #to_s(:format) #44354
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
#to_formatted_s is too cumbersome.
rafaelfranca
reviewed
Feb 8, 2022
There was a problem hiding this comment.
Looks good to me but the tests are not testing the existence of the alias anymore. Someone could come and delete it, which would be a breaking change and we would not know. I commented in a few places that the alias was being tested, but there are more in the body of changes of this PR which I didn't comment.
| # | ||
| # === Examples | ||
| # datetime = DateTime.civil(2007, 12, 4, 0, 0, 0, 0) # => Tue, 04 Dec 2007 00:00:00 +0000 | ||
| # | ||
| # datetime.to_formatted_s(:db) # => "2007-12-04 00:00:00" | ||
| # datetime.to_fs(:db) # => "2007-12-04 00:00:00" | ||
| # datetime.to_fs(:db) # => "2007-12-04 00:00:00" |
There was a problem hiding this comment.
Suggested change
| # datetime.to_fs(:db) # => "2007-12-04 00:00:00" | |
| # datetime.to_formatted_s(:db) # => "2007-12-04 00:00:00" |
| assert_equal "2005-02-21", date.to_fs(:db) | ||
| assert_equal "2005-02-21", date.to_fs(:inspect) | ||
| assert_equal "21 Feb 2005", date.to_fs(:rfc822) | ||
| assert_equal "2005-02-21", date.to_fs(:iso8601) | ||
| assert_equal "21 Feb", date.to_fs(:short) |
There was a problem hiding this comment.
Suggested change
| assert_equal "21 Feb", date.to_fs(:short) | |
| assert_equal "21 Feb", date.to_formatted_s(:short) |
needs to keep testing the alias
| assert_equal "2009-02-05T14:30:05+00:00", DateTime.civil(2009, 2, 5, 14, 30, 5).to_formatted_s(:iso8601) | ||
| assert_equal "2009-02-05T14:30:05-06:00", DateTime.civil(2009, 2, 5, 14, 30, 5, Rational(-21600, 86400)).to_fs(:iso8601) | ||
| assert_equal "2008-06-09T04:05:01-05:00", DateTime.civil(2008, 6, 9, 4, 5, 1, Rational(-18000, 86400)).to_fs(:iso8601) | ||
| assert_equal "2009-02-05T14:30:05+00:00", DateTime.civil(2009, 2, 5, 14, 30, 5).to_fs(:iso8601) | ||
| end | ||
|
|
||
| assert_equal "2005-02-21 14:30:00", datetime.to_fs(:db) |
There was a problem hiding this comment.
Suggested change
| assert_equal "2005-02-21 14:30:00", datetime.to_fs(:db) | |
| assert_equal "2005-02-21 14:30:00", datetime.to_formatted_s(:db) |
|
I'm taking over this PR to fix the test/doc problem. |
rafaelfranca
added a commit
that referenced
this pull request
Feb 8, 2022
Make #to_fs the default replacement for #to_s(:format)
r7kamura
added a commit
to r7kamura/rubocop-rails_deprecation
that referenced
this pull request
Mar 14, 2022
sambostock
added a commit
to Shopify/money
that referenced
this pull request
Mar 17, 2022
rails/rails#44354 introduces `to_fs`, and makes `to_formatted_s` an alias for it, so if we added `to_formatted_s` to match APIs, then we should add `to_fs` as well.
sambostock
added a commit
to Shopify/money
that referenced
this pull request
Mar 17, 2022
Since rails/rails#44354 implements `to_formatted_s` as an alias for `to_fs`, it makes sense for us to do it the same way.
gbp
added a commit
to mysociety/alaveteli
that referenced
this pull request
Apr 11, 2022
Use replacement `#to_fs` instead. Fixes deprecation warnings: `TimeWithZone#to_s(:db) is deprecated. Please use TimeWithZone#to_fs(:db) instead.` See rails/rails#44354
tnir
added a commit
to tnir/forem
that referenced
this pull request
Apr 11, 2022
rails/rails#43772 rails/rails#44354 Signed-off-by: Takuya Noguchi <[email protected]>
2 tasks
jeremyf
pushed a commit
to forem/forem
that referenced
this pull request
Apr 11, 2022
rails/rails#43772 rails/rails#44354 Signed-off-by: Takuya Noguchi <[email protected]>
koic
added a commit
to koic/rails-style-guide
that referenced
this pull request
Apr 19, 2022
Follow up rails/rails#44354. Also, the deprecated warning for `to_s(:db)` is `to_fs(:db)` instead of `to_formatted_s(:db)`. > DateTime#to_s(:db) is deprecated. Please use DateTime#to_fs(:db) instead.
koic
added a commit
to koic/rails-style-guide
that referenced
this pull request
Apr 20, 2022
Follow up rails/rails#44354. Also, the deprecated warning for `to_s(:db)` is `to_fs(:db)` instead of `to_formatted_s(:db)`. > DateTime#to_s(:db) is deprecated. Please use DateTime#to_fs(:db) instead.
gbp
added a commit
to mysociety/alaveteli
that referenced
this pull request
Apr 28, 2022
Use replacement `#to_fs` instead. Fixes deprecation warnings: ``` TimeWithZone#to_s(:db) is deprecated. Please use TimeWithZone#to_fs(:db) instead. ``` See rails/rails#44354
9 tasks
goldapple911
added a commit
to goldapple911/rails-style-guide
that referenced
this pull request
Aug 15, 2023
Follow up rails/rails#44354. Also, the deprecated warning for `to_s(:db)` is `to_fs(:db)` instead of `to_formatted_s(:db)`. > DateTime#to_s(:db) is deprecated. Please use DateTime#to_fs(:db) instead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first take of #to_formatted_s was just too cumbersome for a method used that frequently. It also mixed abbreviation and not in a bit of an unholy mix.