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

Enable extension developers to customize the namespace #3161

Conversation

mdesantis
Copy link
Contributor

@mdesantis mdesantis commented Apr 3, 2019

Description

If you are developing an extension and you want to use Rake::Task['extension:test_app'] you are forced to have a file named exactly as the capitalized version of the extension namespace name, so if you name your namespace extension e.g. SolidusGraphQL you are forced to create a file named solidus_graph_q_l, otherwise the following error is raised:

> bundle exec rake test_app --trace
** Invoke default (first_time)
** Invoke first_run (first_time)
** Execute first_run
** Invoke test_app (first_time)
** Execute test_app
** Invoke extension:test_app (first_time)
** Execute extension:test_app
** Invoke common:test_app (first_time)
** Execute common:test_app
rake aborted!
LoadError: cannot load such file -- solidus_graph_q_l

This change allows the developer to customize the extension namespace setting a LIB_NAMESPACE environment variable.

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)

@mdesantis mdesantis force-pushed the enable-extension-developers-to-customize-the-namespace branch from 9cb6bc7 to f3e564a Compare May 8, 2019 15:12
Copy link
Contributor

@ericsaupe ericsaupe left a comment

Choose a reason for hiding this comment

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

Specs are failing but that might just need a rebase from master

@@ -24,7 +24,7 @@
begin
require "generators/#{ENV['LIB_NAME']}/install/install_generator"
puts 'Running extension installation generator...'
"#{ENV['LIB_NAME'].camelize}::Generators::InstallGenerator".constantize.start(["--auto-run-migrations"])
"#{ENV['LIB_NAMESPACE'] || ENV['LIB_NAME'].camelize}::Generators::InstallGenerator".constantize.start(["--auto-run-migrations"])
Copy link
Contributor

Choose a reason for hiding this comment

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

Does LIB_NAMESPACE need to be camelize as well? Should this be #{(ENV['LIB_NAMESPACE'] || ENV['LIB_NAME']).camelize}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ericsaupe I don't think so, because LIB_NAMESPACE is meant to be the Solidus extension gem namespace, it isn't used as a file path, so it doesn't make sense to pass it in a file path form; I meant to use LIB_NAMESPACE in order to skip transformations, so f.e. in this way:

LIB_NAME=solidus_graphql_api LIB_NAMESPACE=SolidusGraphQLAPI bundle exec rake test_app

Do you think we could refactor in a better way the flow? I was thinking about that but unfortunately it didn't come me to mind anything better

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me! Thanks for the explanation.

@mdesantis mdesantis force-pushed the enable-extension-developers-to-customize-the-namespace branch from f3e564a to 0f73e06 Compare May 9, 2019 12:54
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

👍 Thanks, @mdesantis!

@kennyadsl
Copy link
Member

@mdesantis can you please rebase against master now?

@mdesantis
Copy link
Contributor Author

@kennyadsl FYI I rebased with master a couple of hours ago and surprisingly tests are still failing; I'm going to try again

If you are developing an extension and you want to use
`Rake::Task['extension:test_app']` you are forced to have a file named
exactly as the capitalized version of the extension namespace name, so
if you name your namespace extension e.g. SolidusGraphQL you are forced
to create a file named `solidus_graph_q_l`, otherwise the following
error is raised:

```
> bundle exec rake test_app --trace
** Invoke default (first_time)
** Invoke first_run (first_time)
** Execute first_run
** Invoke test_app (first_time)
** Execute test_app
** Invoke extension:test_app (first_time)
** Execute extension:test_app
** Invoke common:test_app (first_time)
** Execute common:test_app
rake aborted!
LoadError: cannot load such file -- solidus_graph_q_l
```

 This change allows the developer to customize the extension namespace
setting a `LIB_NAMESPACE` environment variable.
@mdesantis mdesantis force-pushed the enable-extension-developers-to-customize-the-namespace branch from 0f73e06 to 8241630 Compare May 9, 2019 14:54
@kennyadsl
Copy link
Member

I've just merged #3202 that should fix the build

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.

3 participants