| Commit message (Collapse) | Author | Age | Files | Lines |
... | |
|\ \
| | |
| | | |
Replace AS::TimeWithZone#since with alias to +
|
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
Stems from [Google group discussion](https://groups.google.com/forum/#!topic/rubyonrails-core/jSPbP-TNLb0).
Currently `AS::TimeWithZone` has two methods to add an interval to a time:
`+(other)` and `since(other)` ([docs](http://edgeapi.rubyonrails.org/classes/ActiveSupport/TimeWithZone.html)).
The two methods are "pretty much" equivalent in every case:
1. When adding any interval to an `AS::TimeWithZone` representing a `Time`:
```ruby
t = Time.now.in_time_zone #=> Thu, 04 Dec 2014 18:56:28 EST -05:00
t + 1 == t.since(1) #=> true
t + 1.day == t.since(1.day) #=> true
t + 1.month == t.since(1.month) #=> true
```
2. When adding any interval to an `AS::TimeWithZone` representing a `Date`:
```ruby
d = Date.today.in_time_zone #=> Thu, 04 Dec 2014 00:00:00 EST -05:00
d + 1 == d.since(1) #=> true
d + 1.day == d.since(1.day) #=> true
d + 1.month == d.since(1.month) #=> true
```
3. When adding any interval to an `AS::TimeWithZone` representing a `DateTime`:
```ruby
dt = DateTime.now.in_time_zone #=> Thu, 04 Dec 2014 18:57:28 EST -05:00
dt + 1 == dt.since(1) #=> true
dt + 1.day == dt.since(1.day) #=> true
dt + 1.month == dt.since(1.month) #=> false
```
As you can see, the only case in which they differ is when the interval added
to a `DateTime` is in a format like `1.month`.
However, this usage of "since" is explicitly discouraged by the
[documentation of `DateTime#since`](https://github.com/rails/rails/blob/master/activesupport/lib/active_support/core_ext/date_time/calculations.rb#L86L88):
> Returns a new DateTime representing the time a number of seconds since the instance time.
> Do not use this method in combination with x.months, use months_since instead!
And indeed, following this recommendation the correct result is returned:
```ruby
dt + 1.month == dt.months_since 1 #=> true
```
Therefore, my proposal is to remove the method definition of `TimeWithZone#since`
and instead replace it with a simple `alias_method :since, :+`.
The rationale is that the only case where they differ is a case that is
explicitly discouraged as "wrong".
In my opinion, having two methods named `since` and `+` and having to figure
out exactly what the difference is makes the codebase more confusing.
However, I understand this PR is "subjective", so if you feel like it's better
to ignore this, feel free to close the PR.
Thanks!
|
|\ \ \
| | | |
| | | |
| | | | |
Use thread_safe gem version greater or equal to 0.3.4
|
| | | | |
|
|/ / /
| | |
| | |
| | |
| | |
| | |
| | |
| | |
| | | |
The previous example was a little convoluted and the exposition
claifying the parts that were correct albeit not totally obvious
were interferring in my view.
This example has less things going on and gets to the key problem
with less balls in the air.
|
|\ \ \
| | | |
| | | | |
Remove unused line
|
| | | | |
|
|\ \ \ \
| |/ / /
|/| | | |
[ci skip] Change three backticks to just one
|
|/ / / |
|
|\ \ \
| |/ /
|/| | |
fix link in autoloading guide [ci skip]
|
|/ / |
|
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
In the case of serialized columns, we would expect the unserialized
value as input, not the serialized value. The original issue which made
this distinction, #14163, introduced a bug. If you passed serialized
input to the method, it would double serialize when it was sent to the
database. You would see the wrong input upon reloading, or get an error
if you had a specific type on the serialized column.
To put it another way, `update_column` is a special case of
`update_all`, which would take `['a']` and not `['a'].to_yaml`, but you
would not pass data from `params` to it.
Fixes #18037
|
|\ \
| | |
| | | |
Add class level case operator support for error dispatching in Rescuable
|
| | | |
|
|\ \ \
| | | |
| | | | |
Improving Method Chaining section [ci skip]
|
|/ / / |
|
|\ \ \
| | | |
| | | |
| | | |
| | | | |
jethroo/fix/assert_template_with_unsupported_layout_type
assert template should raise ArgumentError for unsupported layout types
|
| | | |
| | | |
| | | |
| | | | |
unknown layout type
|
|\ \ \ \
| | | | |
| | | | | |
"backoffice" -> "back office", "lookup" -> "look up" [ci skip]
|
|/ / / /
| | | |
| | | |
| | | |
| | | | |
"Lookup" is a noun, but not a verb. The verb is "look up".
[skip ci]
|
|\ \ \ \
| | | | |
| | | | | |
Add a code checking about file or not to the rails db command
|
| | | | | |
|
|\ \ \ \ \
| | | | | |
| | | | | | |
allow reseting of request variants
|
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
The current implementation of `variants=` don't allow a resetting to nil, wich is the default value.
This results in the following code smell:
```ruby
case request.user_agent
when /iPhone/
request.variants = :phone
when /iPad/
request.variants = :ipad
end
```
With the ability to reset variants to nil, it could be:
```ruby
request.variants = case request.user_agent
when /iPhone/
:phone
when /iPad/
:ipad
end
```
|
|\ \ \ \ \ \
| | | | | | |
| | | | | | | |
Add test for ActiveModel `after_validation`, `after_` and `around_` callbacks returning false
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
This stems from https://github.com/rails/rails/pull/17227#discussion_r21641358
It's simply a clarification of the current behavior by which if an
`after_` or `around_` ActiveModel callback returns +false+, then the callback
chain **is not halted**.
The callback chain in ActiveModel is only halted when a `before_`
callback returns `false`.
|
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
This stems from https://github.com/rails/rails/pull/17227#discussion_r21641358
It's simply a clarification of the current behavior by which if an
`after_validation` ActiveModel callback returns +false+, then further
`after_` callbacks **are not halted**.
|
|\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | | |
Test to ensure content_tag works when fixing #17661
|
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
After c2fe093, which was reverted yesterday, there will likely be
future PRs to address #17661.
The test `actionview/test/template/capture_helper_test.rb
#test_capture_returns_nil_if_the_returned_value_is_not_a_string` should
have errored with c2fe093, but it was rewritten in the PR to not raise.
My guess is that it may have seemed irrelevant to the content_tag tests
or already covered by them.
This test provides additional protection by being in the content_tag
test suite to explicitly raise a red flag in future cases. It foregoes
some redundancy for safety — at least until #17661 is closed.
|
|\ \ \ \ \ \ \ \
| |/ / / / / / /
|/| | | | | | | |
Add test for `:skip_after_callbacks_if_terminated`
|
| |/ / / / / /
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | |
| | | | | | | |
`define_callbacks` from `ActiveSupport::Callbacks` accepts the
`:skip_after_callbacks_if_terminated` option since #4866 but the option
is not tested anywhere.
This commit adds tests and fixes documentation for the option, making it clear
that halting a callback chain only stops following `before_` and `around_`
callbacks by default.
|
|\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | | |
Improve section in constant autoload guide
|
| | |/ / / / /
| |/| | | | |
| | | | | | | |
[skip ci]
|
|\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
claudiob/remove-redundant-tests-about-around-actions-returning-false
Remove misleading test: around_action return false
|
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
When an `around_action` does not `yield`, then the corresponding action is
*never* executed and the `after_` actions are *never* invoked.
The value returned by the `around_action` does not have any impact on this:
an `around_action` can "return" `true`, `false`, or `"pizza"`, but as long
as `yield` is not invoked, the corresponding action and after callbacks are
not executed.
The test suite for `ActionController::Callbacks` currently includes separate
tests to distinguish the cases in which a non-yielding `around_actions` returns
`true` or `false`.
In my opinion, having such tests is misleading, giving the impression that the
returned value might have some sort of impact, while it does not. At least
that's the impression I got when I read those tests.
For completeness, the tests were introduced 7 years ago by @NZKoz in e80fabb.
|
|\ \ \ \ \ \ \ \
| | | | | | | | |
| | | | | | | | | |
English fix [ci skip]
|
|/ / / / / / / / |
|
|\ \ \ \ \ \ \ \
| | | | | | | | |
| | | | | | | | | |
Improve grammar
|
|/ / / / / / / /
| | | | | | | |
| | | | | | | | |
[skip ci]
|
|\ \ \ \ \ \ \ \
| |_|/ / / / / /
|/| | | | | | | |
Constant autoload grammar fix
|
|/ / / / / / /
| | | | | | |
| | | | | | | |
[skip ci]
|
| | | | | | | |
|
|\ \ \ \ \ \ \
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
JuanitoFatas/patch/constant_autoloading_and_reloading-list-items
[ci skip] :scissors: blank lines between list items.
|
| | |_|/ / / /
| |/| | | | | |
|
|/ / / / / /
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
[ci skip]
/cc @sgrif @zzak
Conflicts:
activerecord/CHANGELOG.md
|
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
I think "What" is was fine, "The purpose of" assumes the reader knows such thing exists.
"What is" is fine, and also matches the following "What is require_dependency".
|
| | | | | | |
|
| | | | | | |
|
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | |
| | | | | | |
4.2.0", then they should have no problems hacking with Rails. [ci skip]
If they do, then its a bug and it should be reported.
|
|\ \ \ \ \ \
| | | | | | |
| | | | | | | |
Improving tips about installation in the guides [ci skip]
|