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

Fix install generator on namespaced extensions #4327

Merged
merged 1 commit into from
Apr 6, 2022
Merged

Fix install generator on namespaced extensions #4327

merged 1 commit into from
Apr 6, 2022

Conversation

nvandoorn
Copy link
Contributor

@nvandoorn nvandoorn commented Apr 1, 2022

Description

Prior to #4302, camelize was called on the
LIB_NAME value which converts / to ::. This behaviour worked for
namespaced extensions because we were invoking the class directly,
instead of through the bin/rails bin stub.

The change to run the generator through the binstub breaks for extensions with a LIB_NAME like “super_good/solidus_taxjar” because Rails will not be able to find he task correctly.

To fix this, we convert to a "rake style" namespace by replacing /
with :.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

Prior to #4302, `camelize` was called on the
`LIB_NAME` value which converts `/` to `::`. This behaviour worked for
namespaced extensions because we were invoking the class directly,
instead of through the `bin/rails` bin stub.

To fix this, we convert to a "rake style" namespace by replacing `/`
with `:`.

Co-authored-by: Chris Todorov <chris@super.gd>
Co-authored-by: Adam Mueller <adam@super.gd>
Copy link
Contributor

@waiting-for-dev waiting-for-dev left a comment

Choose a reason for hiding this comment

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

Thanks!

@kennyadsl
Copy link
Member

kennyadsl commented Apr 6, 2022

@gsmendoza tagging you for visibility!

Thanks @nvandoorn and @adammathys!

@kennyadsl kennyadsl merged commit dddc570 into solidusio:master Apr 6, 2022
@gsmendoza
Copy link
Contributor

@nvandoorn Thanks for fixing the namespace! :)

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.

5 participants