Followup to #1158 #1168
Followup to #1158 #1168
Conversation
`has_many` will check the same thing
1. Users might (should) pass a frozen hash to us 2. Even if they don't, users will not expect their hash to be modified
jaredbeck
force-pushed
the
followup_1158
branch
from
372b4ba to
1c06eed
Compare
|
@TylerRick a few minor follow-ups to your #1158 plz review thanks d241663 and 1c06eed are the more interesting commits, 66fba21 is just code style |
| table_name through validate | ||
| ] | ||
| end | ||
| end |
There was a problem hiding this comment.
Good catch. 👍 Sorry, I sure thought I'd removed that already! Maybe it got added back by mistake when I did a rebase.
There was a problem hiding this comment.
Oh, I see it now. I'd forgotten to push up that commit (pushed now).
| @@ -21,11 +21,12 @@ class ModelConfig | |||
| DPR_PASSING_ASSOC_NAME_DIRECTLY_TO_VERSIONS_OPTION = <<~STR.squish | |||
| Passing versions association name as `has_paper_trail versions: %{versions_name}` | |||
| is deprecated. Use `has_paper_trail versions: {name: %{versions_name}}` instead. | |||
| The hash you pass to `versions:` is now passed directly to `has_many`. | |||
There was a problem hiding this comment.
That's not actually true in the case of :name and :class_name.
There was a problem hiding this comment.
It seems odd to state the general rule in the warning message for the exact 2 cases that are the exceptions to that rule. :) The general rule is documented in the Readme; the only thing they need to know in the deprecation warning is what to change it to, which is already covered.
| # - :versions - Either, | ||
| # - A String (deprecated) - The name to use for the versions | ||
| # association. Default is `:versions`. | ||
| # - A Hash - options passed to `has_many`, plus `name:` and `scope:`. |
There was a problem hiding this comment.
Good catch! Forgot about this documentation change.
No description provided.