Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rails 7.0 Compatibility #1365

Merged
merged 15 commits into from
Jan 21, 2022
Merged

Rails 7.0 Compatibility #1365

merged 15 commits into from
Jan 21, 2022

Conversation

dfurber
Copy link
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
@colinxfleming colinxfleming mentioned this pull request Jan 18, 2022
Closed
mathieujobin
mathieujobin approved these changes Jan 19, 2022
View reviewed changes
Copy link
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
Contributor

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
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.

jaredbeck
jaredbeck approved these changes Jan 20, 2022
View reviewed changes
Copy link
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

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
Contributor

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

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

@t27duck
Copy link
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

@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

@mathieujobin mathieujobin mathieujobin approved these changes

@jaredbeck jaredbeck jaredbeck approved these changes

@pedrocarmona pedrocarmona pedrocarmona approved these changes

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