-
-
Notifications
You must be signed in to change notification settings - Fork 76
Make extension compliant to solidus_dev_support #289
Conversation
f21a5cd
to
8e08a23
Compare
@MinasMazar this is still needed after merging #286 ? |
This could great to move forward @kennyadsl & @MinasMazar, not having a sandbox on this extension makes developing quite a pain 🙁 I am happy to help out if needed 🙂 |
hey sorry, this work has been stale for a long time :\ @kennyadsl @RyanofWoods if I recall correctly #286 was only addressing some changes required for Solidus 3.0 concerning the |
01cade6
to
80696bb
Compare
@@ -0,0 +1,4 @@ | |||
# frozen_string_literal: true | |||
|
|||
FactoryBot.define do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the existing factories should be moved into here @MinasMazar?
https://github.com/solidusio/solidus_paypal_braintree/blob/master/lib/solidus_paypal_braintree/factories.rb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! Updated here.
0c28d09
to
6f37371
Compare
Thanks @MinasMazar, if you can squash the fixup I think we are there! |
6f37371
to
f53fe59
Compare
Thanks @MinasMazar! 🙌 |
Make extension compliant to latest solidus_dev_support.
Adhere the extension to dev support template via
solidus extension .
and resolve all conflicts.N.B. we're still having some problems with the sandbox generator script (
bin/sandbox
). This will be addressed in a separate PR.Some test are failing: comparing the latest build from master and the one for this PR, I see the same errors. There's already a PR to fix them (#328); on my local env I've temporarily rebased this PR on that one: all tests passed, so I assume I haven't introduced any new bug.