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.

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

add support for rails v8 #521

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jamesoneill997
Copy link

Noticed some issues when trying to integrate v6.4.4 with rails v8, hoping that adding support here should resolve things.

Haven't contributed to this repo before, so please feel free to let me know if I've missed anything.

@jamesoneill997
Copy link
Author

Hey @smaboshe // @sarahraqueld ,

Any ideas on a timeline for getting a review on this? I'm more than happy to make any requested changes to get this one working and over the line.

@smaboshe smaboshe requested a review from DoodlingDev January 15, 2025 10:42
@neilvcarvalho neilvcarvalho self-requested a review January 21, 2025 12:23
Copy link
Member

@neilvcarvalho neilvcarvalho left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

Just adding Rails 8.0 to the test matrix won't solve the issues you're having. It could surface them with broken specs, but looks like they're all passing. Would you mind creating an issue describing what you were facing and what workarounds you did, if any?

@@ -3,6 +3,10 @@ there might not be any notable changes in new versions of this project.

# NEWS

## 6.4.5 (December 6, 2024)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## 6.4.5 (December 6, 2024)

We're going to add the version number and release date when this is released. In the meantime, we just add to the top as unreleased.

@@ -1,3 +1,3 @@
module FactoryBotRails
VERSION = "6.4.4".freeze
VERSION = "6.4.5".freeze
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
VERSION = "6.4.5".freeze
VERSION = "6.4.4".freeze

We're going to create a dedicated commit to the version change when this is released.

@@ -1,7 +1,7 @@
PATH
remote: .
specs:
factory_bot_rails (6.4.4)
factory_bot_rails (6.4.5)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
factory_bot_rails (6.4.5)
factory_bot_rails (6.4.4)

@jamesoneill997
Copy link
Author

jamesoneill997 commented Jan 23, 2025

Thanks for the review @neilvcarvalho ! Yeah I was speculatively hoping that this might nudge the issue we are seeing in the right direction, but I don't see it resolving it.

I'm just working through some backlog atm, so bear with me, it might be a few days before I get a chance to recreate and make an issue. From my recollection, I was noticing some validation errors were missing when bumping to rails 8, and I had traced it back to this gem. I'll get a proper issue made up in the coming days and add as much detail as I can. Will also tidy this up as per your comments. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants