Skip to content

Rails 7.0 Compatibility#1365

Merged
jaredbeck merged 15 commits intopaper-trail-gem:masterfrom
dfurber:master
Jan 21, 2022
Merged

Rails 7.0 Compatibility#1365
jaredbeck merged 15 commits intopaper-trail-gem:masterfrom
dfurber:master

Conversation

@dfurber
Copy link
Copy Markdown
Contributor

@dfurber dfurber commented Jan 15, 2022 •
edited
Loading

Makes PaperTrail work with Rails 7. The specs now pass, with underlying issues addressed and comments added. You'll have to add it to the GitHub Actions.

Most of this PR is actually just adding specs unrelated to Rails 7 to get the coverage threshold up over the line.

@dfurber dfurber marked this pull request as draft January 16, 2022 01:59
@dfurber dfurber marked this pull request as ready for review January 16, 2022 02:08
pedrocarmona
pedrocarmona reviewed Jan 17, 2022
View reviewed changes
Comment thread lib/paper_trail/version_concern.rb Outdated
@colinxfleming colinxfleming mentioned this pull request Jan 18, 2022
Closed
mathieujobin
mathieujobin approved these changes Jan 19, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@mathieujobin mathieujobin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@mathieujobin
Copy link
Copy Markdown
Contributor

mathieujobin commented Jan 19, 2022

Thank you very much for this, I am using this branch now and its working well.

@mathieujobin mathieujobin mentioned this pull request Jan 19, 2022
Closed
6 tasks
jaredbeck
jaredbeck reviewed Jan 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jaredbeck jaredbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @dfurber long time no see! Thanks for the PR. I love to see all the new tests, and the great comments.

The only thing I'm concerned about is the instance_variable_set. I'm hoping @pedrocarmona's research will lead to solution that uses public API.

Comment thread lib/paper_trail.rb
Comment thread lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb
Comment thread lib/paper_trail/version_concern.rb Outdated
Comment thread lib/paper_trail/version_number.rb Outdated
jaredbeck
jaredbeck approved these changes Jan 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@jaredbeck jaredbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks David!

@pedrocarmona , any concerns?

@pedrocarmona
Copy link
Copy Markdown

pedrocarmona commented Jan 20, 2022

LGTM, thanks David!

@pedrocarmona , any concerns?

LGTM

Thank you @dfurber

@jaredbeck jaredbeck merged commit c10a857 into paper-trail-gem:master Jan 21, 2022
@mathieujobin
Copy link
Copy Markdown
Contributor

mathieujobin commented Jan 24, 2022

oups, I think we are missing the update to the .github/workflows "script"

@mathieujobin mathieujobin mentioned this pull request Jan 24, 2022
Merged
6 tasks
@baarkerlounger
Copy link
Copy Markdown

baarkerlounger commented Feb 7, 2022

Now that this is merged would it be possible to cut a new release?

@t27duck
Copy link
Copy Markdown
Contributor

t27duck commented Feb 7, 2022

Now that this is merged would it be possible to cut a new release?

12.2.0 is that release. Been out for a couple weeks.

@baarkerlounger
Copy link
Copy Markdown

baarkerlounger commented Feb 7, 2022

@t27duck thanks, missed that as Github releases still shows 12.0.0.

@chubchenko chubchenko mentioned this pull request Mar 8, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jaredbeck jaredbeck jaredbeck approved these changes

+2 more reviewers

@mathieujobin mathieujobin mathieujobin approved these changes

@pedrocarmona pedrocarmona pedrocarmona approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@dfurber @mathieujobin @pedrocarmona @baarkerlounger @t27duck @jaredbeck @dfurber-truecar