-
Notifications
You must be signed in to change notification settings - Fork 23
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 solidus_easypost file #32
add solidus_easypost file #32
Conversation
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.
Thanks a lot for this PR, @peterberkenbosch ! 🙌
But I have a few questions:
-
Wouldn't it be better if we renamed this file accordingly?
-
If that's the case, shouldn't we rename the folder
spree_easypost
underlib/
too? -
It seems that this extension is missing a
version.rb
file underlib/spree_easypost/
, what do you think about adding said file on this PR as well?
yes, I agree @aitbw, this was the quick fix mode but the best is to rename it. I'll add the version file too. |
@peterberkenbosch OMG. Awesome! Thank you! |
@aitbw renamed all and introduced version file. Decided to go to a |
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.
Fantastic work, @peterberkenbosch ❤️ 👏 thanks a lot for this patch! 🙇♀️
What do you think on this, @kennyadsl @jacobherrington ? |
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.
Thanks @peterberkenbosch, just left a suggestion 👍
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.
Thanks @peterberkenbosch!
Let's wait to reach (and fix) a consensus on solidusio/solidus#3074 before merging this. |
Re-running specs now that solidusio/solidus#3078 and solidusio/solidus#3079 are merged. |
Done with #33, @peterberkenbosch 👌 |
the extension is expected to have a solidus_easypost file to load the engine and other libraries. This commit introduces that file that is a simple wrapper around the existing spree_easypost file. This fixes the problem with not loading migrations.
30b3830
to
732eccd
Compare
Re-running specs due to a Travis failure. |
the extension is expected to have a
solidus_easypost
file to load theengine and other libraries. This commit introduces that file that is
a simple wrapper around the existing
spree_easypost
file.This fixes the problem with not loading migrations. ( fixes #30 )