-
-
Notifications
You must be signed in to change notification settings - Fork 37
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
Discourage SolidusStarterFrontend from being run as an engine #167
Changes from 3 commits
a4600e2
08ad480
cfc6371
14893fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'solidus_dev_support/rake_tasks' | ||
|
||
ENV['SOLIDUS_STARTER_FRONTEND_ALLOW_AS_ENGINE'] = 'true' | ||
SolidusDevSupport::RakeTasks.install | ||
|
||
task default: 'extension:specs' |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,17 @@ Simply add this require statement to your spec_helper: | |
require 'solidus_starter_frontend/factories' | ||
``` | ||
|
||
### Running the extension in development | ||
|
||
In order to run the extension, you can either 1) run the sandbox script to | ||
generate the sandbox app or 2) add the extension as a gem in a Rails app. | ||
|
||
### Running the sandbox | ||
gsmendoza marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
The sandbox script uses the `solidus_starter_frontend:install` generator to | ||
install the frontend. This is useful when you want to ensure that the generator | ||
is working. | ||
|
||
To run this extension in a sandboxed Solidus application, you can run | ||
`bin/sandbox`. The path for the sandbox app is `./sandbox` and `bin/rails` will | ||
forward any Rails commands to `sandbox/bin/rails`. | ||
|
@@ -42,6 +52,82 @@ Use Ctrl-C to stop | |
|
||
Default username and password for admin are: `[email protected]` and `test123`. | ||
|
||
### Running the extension as an engine in a Rails app | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As I mentioned in the video, one option is to create a script to automatically run these commands. Let me know what you think! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I don't fully understand why we need to provide two different development options. Shouldn't it be enough to have the sandbox load There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @waiting-for-dev The two development options have different goals:
Previously, the sandbox script was both loading the gem and generating the frontend. In #166, I changed the sandbox script to fulfill only goal 1, based on the reasoning that the gem will be used only as a generator in actual usage. Goal 2, on the other hand, is more of a nice-to-have for the gem contributor, since it just makes it easier for the contributor to preview changes they make to the app code. We could change the sandbox script to load the gem instead of using it to generate the frontend. Perhaps I might be misunderstanding what a sandbox app is supposed to do, based on conventions we follow with other Solidus extensions? Your thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmmm yeah, it makes sense. Ideally, I'd like to confirm that the
What do you mean by the conventions we follow with other extensions? I haven't worked with Solidus extensions, so I'm unfamiliar. Isn't the sandbox app there picking changes made to the extension code? At least, it's what I do on the main Solidus repo, so I had the idea it was one of its purposes. Anyway, yeah, probably we need both strategies, but I definitely would automate both of them. Maybe we could even provide a flag to I'd like to ping @kennyadsl to hear what are his thoughts on this, too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @waiting-for-dev I agree with defaulting to as-gem version since 1) this seems to be the more common way you'd want to use the sandbox, and 2) based on what you said, it's how the sandbox behaves in the main Solidus repo. I also like the idea of have two different scripts/apps for
gsmendoza marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
You can also run the extension as a engine in a Rails app. This is useful when | ||
you want to automatically see changes in the app without having to rerun the | ||
`solidus_starter_frontend:install` generator. | ||
|
||
To run the extension in a Rails app, | ||
|
||
#### 1) Create a Rails app | ||
|
||
```sh | ||
rails new store | ||
``` | ||
|
||
#### 2) Add and install Solidus with `solidus_starter_frontend` in the Rails app | ||
|
||
Add the following gems. Since we're developing `solidus_starter_frontend`, we're | ||
assuming that you have `solidus_starter_frontend` locally and that you want to | ||
point to the latest versions of the Solidus components and extensions. | ||
|
||
```rb | ||
solidus_repo = ENV.fetch('SOLIDUS_REPO', 'solidusio/solidus') | ||
solidus_branch = ENV.fetch('SOLIDUS_BRANCH', 'master') | ||
gem 'solidus_core', github: solidus_repo, branch: solidus_branch | ||
gem 'solidus_api', github: solidus_repo, branch: solidus_branch | ||
gem 'solidus_backend', github: solidus_repo, branch: solidus_branch | ||
gem 'solidus_sample', github: solidus_repo, branch: solidus_branch | ||
|
||
gem 'solidus_starter_frontend', path: 'path/to/solidus_starter_frontend' | ||
|
||
solidus_auth_devise_repo = ENV.fetch('SOLIDUS_AUTH_DEVISE_REPO', 'solidusio/solidus_auth_devise') | ||
solidus_auth_devise_branch = ENV.fetch('SOLIDUS_AUTH_DEVISE_BRANCH', 'master') | ||
gem 'solidus_auth_devise', github: solidus_auth_devise_repo, branch: solidus_auth_devise_branch | ||
``` | ||
|
||
Note that `solidus_starter_frontend` has conditional references to | ||
`solidus_auth_devise`, so it's important to place `solidus_starter_frontend` | ||
before `solidus_auth_devise`. | ||
|
||
#### 3) Enable `SOLIDUS_STARTER_FRONTEND_ALLOW_AS_ENGINE` environment variable | ||
|
||
We're strongly encouraging people to use `solidus_starter_frontend` as a | ||
generator. Thus, if you want to run it as an engine, you'll need to set an | ||
environment variable explicitly stating so. | ||
|
||
One way to do this is to set the environment variable within an initializer: | ||
|
||
```sh | ||
# config/initializers/solidus_starter_frontend.rb | ||
ENV['SOLIDUS_STARTER_FRONTEND_ALLOW_AS_ENGINE'] = 'true' | ||
``` | ||
Comment on lines
+102
to
+105
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if there's a better way to do this. Setting |
||
|
||
#### 3) Install Solidus | ||
|
||
```sh | ||
bundle exec rails generate solidus:install \ | ||
--auto-accept \ | ||
--user_class=Spree::User \ | ||
--enforce_available_locales=true \ | ||
--with-authentication=false \ | ||
--payment-method=none | ||
|
||
bundle exec rails generate solidus:auth:install --auto-run-migrations | ||
``` | ||
|
||
#### 4) Install the frontend assets | ||
|
||
You will need to run `bundle exec rails g solidus_starter_frontend:install` to add the | ||
frontend assets to the existing vendored Solidus manifest files. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just realized now that instead of running this install command when running the gem as an engine, we could add the assets directly to |
||
|
||
#### 5) Run the server | ||
|
||
```sh | ||
rails server | ||
``` | ||
|
||
### Updating the changelog | ||
Before and after releases the changelog should be updated to reflect the | ||
up-to-date status of the project: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,8 @@ class Engine < Rails::Engine | |
end | ||
|
||
config.to_prepare do | ||
raise 'Error: SolidusStarterFrontend is not allowed to run as an engine' unless ENV['SOLIDUS_STARTER_FRONTEND_ALLOW_AS_ENGINE'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I appreciate your suggestions on improving the copy of this error message. |
||
|
||
if defined?(Spree::Auth::Engine) | ||
[ | ||
Spree::UserConfirmationsController, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
# Configure Rails Environment | ||
ENV['RAILS_ENV'] = 'test' | ||
ENV['SOLIDUS_STARTER_FRONTEND_ALLOW_AS_ENGINE'] = 'true' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is needed when running |
||
|
||
# Run Coverage report | ||
require 'solidus_dev_support/rspec/coverage' | ||
|
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.
This is needed when running
bundle exec rake
.