Support for activerecord-typedstore v1.5.0 #976
Conversation
etiennebarrie
force-pushed
the
activerecord-typedstore-v1.5.0
branch
from
7789751
to
ccbc44a
Compare
| store_data.accessors.each do |accessor, name| | ||
| field = store_data.fields.fetch(accessor) |
There was a problem hiding this comment.
Does this change means we're not going to be retro-compatible with activerecord-typedstore < 1.5.0?
There was a problem hiding this comment.
No fields is still a Hash. Previously we got a NoMethodError type_sym, using fetch would have shown the error at the moment it was the most logical.
Sorry I missed #949. I think it's missing some things, I'll comment there, but feel free to close this PR here if that makes more sense.
There was a problem hiding this comment.
But this change here going from fields[accessor] to fields.fetch(accessor) is not strictly necessary.
|
Copied the test from @KaanOzkan testing against 1.4.0. |
Otherwise you get "Resolving dependencies..." sometimes as the first line.
For speed reasons, the project is only setup once, but if a gem is added, it can leak to other tests. This saves Gemfile and Gemfile.lock before all `generate` tests, and restores it before each of the test, which avoids re-creating the whole project, or even just running `bundle install` which takes a couple of seconds.
|
So I got some flakiness with this commit, where installing the |
I like this approach and I think it is fine having it inside this PR. I think we can extract that to the general
This is also a good fix and I think we can make this better later as well, e.g. by running |
Motivation
The internals changed in byroot/activerecord-typedstore#91 and
store.accessorsis now a Hash instead of an Array. The value is the name of the method for which we need to compile the RBI.Implementation
I implemented it in a way that should be compatible with the previous version.
Instead of
flat_map(&:accessors).empty?, which used to give[]but now gives[{}]if a store has no field, I useall?, and I use the fact that Hash objects yield two values toeachto preserve the previous behavior.Tests
I made the changes first without the gem bump, to show it's still the same behavior, then the gem bump and the additional test for the new feature.
Closes #949