Support polymorphic has many

westonganger / paper_trail-association_tracking

Plugin for the PaperTrail gem to track and reify associations

MIT License

128 stars 38 forks source link

Support polymorphic has many #10

Closed batistadasilva closed 5 years ago

batistadasilva commented 5 years ago

Hi again, I have been doing some more work on polymorphic associations

Added a test case for reify belongs_to polymorphic association (I didn't do at last PR, my bad)
Fix reify polymorphic has_many association (with test case)

:wink:

westonganger commented 5 years ago

v1.1.0 has now been released which contains these changes.

jaredbeck commented 5 years ago

v1.1.0 has now been released which contains these changes.

If I understand this PR correctly, it requires the database to contain a new column, foreign_type. Is that a fair characterization? If so, this seems to me like a breaking change. Certainly, it broke the PT test suite, and I have changed the PT-AT constraint accordingly.

-  s.add_development_dependency "paper_trail-association_tracking", "< 2"
+  s.add_development_dependency "paper_trail-association_tracking", "< 1.1"

So, perhaps this release should have been 2.0?

Going forward, what PT-AT constraint would you like to see in the PT gemspec? The discussion regarding said constraint could happen here, or at https://github.com/paper-trail-gem/paper_trail/pull/1174 if you prefer.

westonganger commented 5 years ago

Sorry, yeah I will resolve this and go to v2.0.0 instead and I will also probably yank v1.1.0. I will do this just as soon as the fixes in #11 is verified

westonganger commented 5 years ago

From now on I will be following the semantic versioning more closely with this. However if you desire to restrict the PT gemspec further, you could possibly switch to restricting PT-AT by minor version number and I can submit a PR whenever PT-AT minor versions are changed.

jaredbeck commented 5 years ago

.. and I will also probably yank v1.1.0

I'd prefer you didn't yank it, because the PT build depends on it right now. I think we're only supposed to use yank in really bad situations like discovering malware.

From now on I will be following the semantic versioning more closely with this. However if you desire to restrict the PT gemspec further, you could possibly switch to restricting PT-AT by minor version number and I can submit a PR whenever PT-AT minor versions are changed.

Cool. Yeah, if you're going to follow SemVer more closely, then I'm fine with a constraint like ~> 2.0, if that's what you want to do. Or, if you prefer ~> 2.0.0 that's fine too. Please open a PR with whatever you decide.

westonganger commented 5 years ago

v2.0.0 is now released.

jaredbeck commented 5 years ago

v2.0.0 is now released.

Thanks, Weston! The current constraint in PT master is:

add_development_dependency "paper_trail-association_tracking", "~> 1.1.0"

Please open a PR if you would like that to change, thanks!