Conversation
|
Can someone rerun those checks? Seems like a dependency / gems server was temporarily unavailable |
|
@mswiszcz Yup! Re-running now. |
| end | ||
|
|
||
| def foreign_type_matches? | ||
| return true unless options.key?(:foreign_type) |
There was a problem hiding this comment.
We don't typically return early or use the modifier form of unless. Since the end result is a boolean, what do you think about combining these into one conditional?
!options.key?(:foreign_type) || (
!belongs_foreign_type_missing? &&
!has_foreign_type_missing?
)| end | ||
|
|
||
| def foreign_type | ||
| type = if [:has_one, :has_many].include?(macro) |
There was a problem hiding this comment.
To simplify this expression and remove the temporary assignment, what do you think about moving the .to_s to has_column? at line 1509?
| end | ||
|
|
||
| it 'accepts an association with a nonstandard type, with reverse association turned off' do | ||
| define_model :visitor, location_id: :integer, facitility_type: :string |
There was a problem hiding this comment.
Should this be facility_type?
|
I think this would be a nice addition, @mswiszcz do you still plan to address the feedback? If not I can address it and give you credits. |
|
hey @matsales28, I totally forgot about that PR after changing project, sorry about that. Please feel free to take that over as I don't have much time at the moment to finish it |
|
I'm closing this PR in favor of #1609 |
Allows to create tests checking set foreign_type