-
-
Notifications
You must be signed in to change notification settings - Fork 629
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 more detailed description when adding --help option to generator. #161
Changes from 3 commits
ddc4a08
efee64f
e99a3c4
aa60078
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 |
---|---|---|
@@ -0,0 +1,125 @@ | ||
Description: | ||
|
||
The react_on_rails:install generator combined with the example pull requests of | ||
generator runs will get you up and running efficiently. There's a fair bit of | ||
setup with integrating Webpack with Rails. Defaults for options are such that | ||
the default is for the flag to be off. For example, the default for -R is that | ||
redux is off, and the default of -b means that skip-bootstrap is off. | ||
|
||
* Redux | ||
|
||
If you have used the --redux generator option, you will notice the familiar | ||
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 perhaps instead of lines 11 through 13, we could go with:
|
||
additional redux folders. The Hello World example has also been modified to | ||
use Redux. | ||
|
||
Note the organizational paradigm of "bundles". These are like application | ||
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. Periods should go inside of the quotations, so 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. We should start this line off by saying "The generator uses |
||
domains and are used for grouping your code into webpack bundles, in case | ||
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. no comma, this is not an independent clause |
||
you decide to create different bundles for deployment. This is also useful | ||
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. @justin808 is this really the reason we use bundles? I thought it was more of an organizational thing as described below? |
||
for separating out logical parts of your application. The concept is that | ||
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. " |
||
each bundle will have it's own Redux store. If you have code that you want | ||
to reuse across bundles, including components and reducers, place them | ||
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. "across bundles, such as middleware or common utilities, place them" |
||
under /client/app/lib. | ||
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. You can then import them in your client code: |
||
|
||
* Using Images and Fonts | ||
|
||
The generator has amended the folders created in `client/assets/` to Rails's | ||
asset path. We recommend that if you have any existing assets that you want | ||
to use with your client code, you should move them to these folders and use | ||
webpack as normal. This allows webpack's development server to have access | ||
to your assets, as it will not be able to see any assets in the default Rails | ||
directories which are above the `/client` directory. | ||
|
||
Alternatively, if you have many existing assets and don't wish to move them, | ||
you could consider creating symlinks from `client/assets` that point to your | ||
Rails assets folders inside of `app/assets/`. The assets there will then be | ||
visible to both Rails and webpack. | ||
|
||
* Bootstrap Integration | ||
|
||
React on Rails ships with Twitter Bootstrap already integrated into the build. | ||
Note that the generator removes require_tree in both the application.js and | ||
application.css.scss files. This is to ensure the correct load order for the | ||
bootstrap integration, and is usually a good idea in general. You will therefore | ||
need to explicitly require your files. | ||
|
||
How the Bootstrap library is loaded depends upon whether one is using the Rails | ||
server or the HMR development server. | ||
|
||
1. Bootstrap via Rails Server | ||
|
||
In the former case, the Rails server loads bootstrap-sprockets, provided | ||
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. " |
||
by the bootstrap-sass ruby gem (added automatically to your Gemfile by | ||
the generator) via the `app/assets/stylesheets/_bootstrap-custom.scss` | ||
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. "generator), via the" |
||
partial. | ||
|
||
This allows for using Bootstrap in your regular Rails stylesheets. If you | ||
wish to customize any of the Bootstrap variables, you can do so via the | ||
`client/assets/stylesheets/_pre-bootstrap.scss` partial. | ||
|
||
2. Bootstrap via Webpack Dev Server | ||
|
||
When using the webpack dev server, which does not go through Rails, | ||
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. Try to avoid passive voice. Replace 61 thorugh 63 with: "The webpack dev server does not go through Rails but instead loads bootstrap via the |
||
bootstrap is loaded via the `bootstrap-sass-loader` which uses the | ||
`client/bootstrap-sass-config.js` file. | ||
|
||
3. Keeping Custom Bootstrap Configurations Synced | ||
|
||
Because the webpack dev server and Rails each load Bootstrap via a different | ||
file (explained in the two sections immediately above), any changes to | ||
the way components are loaded in one file must also be made to the other | ||
file in order to keep styling consistent between the two. For example, | ||
if an import is excluded in _bootstrap-custom.scss, the same import should | ||
be excluded in `bootstrap-sass-config.js` so that styling in the Rails | ||
server and the webpack dev server will be the same. | ||
|
||
4. Skip Bootstrap Integration | ||
|
||
Bootstrap integration is enabled by default, but can be disabled by passing | ||
the --skip-bootstrap flag (alias -b). When you don't need Bootstrap in your | ||
existing project, just skip it as needed. | ||
|
||
* Linters | ||
|
||
The React on Rails generator can add linters and their recommended accompanying | ||
configurations to your project. There are two classes of linters: ruby linters | ||
and JavaScript linters. | ||
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. @justin808 I think we can safely delete lines 80 through 85 without eliminating any information. |
||
|
||
* JavaScript Linters | ||
|
||
JavaScript linters are enabled by default, but can be disabled by passing the | ||
--skip-js-linters flag (alias j), and those that run in Node have been add to | ||
`client/package.json` under devDependencies. | ||
|
||
* Ruby Linters | ||
|
||
Ruby linters are disabled by default, but can be enabled by passing the | ||
`--ruby-linters` flag when generating. These linters have been added to your | ||
Gemfile in addition to the the appropriate Rake tasks. | ||
|
||
We really love using all the linters! Give them a try. | ||
|
||
* Running the Linters | ||
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. @justin808 I don't think we should include information about how to actually run the linters here. This is basically just parroting what the |
||
|
||
To run the linters (runs all linters you have installed, even if you installed | ||
both Ruby and Node): | ||
|
||
`rake lint` | ||
|
||
Run this command to see all the linters available | ||
|
||
`rake -T lint` | ||
|
||
Here's the list: | ||
========================================================== | ||
rake lint # Runs all linters | ||
rake lint:eslint # eslint | ||
rake lint:js # JS Linting | ||
rake lint:jscs # jscs | ||
rake lint:rubocop[fix] # Run Rubocop lint in shell | ||
rake lint:ruby # Run ruby-lint as shell | ||
rake lint:scss # See docs for task 'scss_lint' | ||
========================================================== | ||
|
||
More Details: | ||
|
||
`https://github.com/shakacode/react_on_rails#generator` | ||
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. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,42 +3,44 @@ | |
module ReactOnRails | ||
module Generators | ||
class InstallGenerator < Rails::Generators::Base | ||
# fetch USAGE file for details generator description | ||
source_root(File.expand_path("../", __FILE__)) | ||
|
||
# --redux | ||
class_option :redux, | ||
type: :boolean, | ||
default: false, | ||
desc: "Install Redux gems and Redux version of Hello World Example", | ||
desc: "Install Redux gems and Redux version of Hello World Example. Default: false", | ||
aliases: "-R" | ||
# --server-rendering | ||
class_option :server_rendering, | ||
type: :boolean, | ||
default: false, | ||
desc: "Add necessary files and configurations for server-side rendering", | ||
desc: "Add necessary files and configurations for server-side rendering. Default: false", | ||
aliases: "-S" | ||
# --skip-js-linters | ||
class_option :skip_js_linters, | ||
type: :boolean, | ||
default: false, | ||
desc: "Skip installing JavaScript linting files", | ||
desc: "Skip installing JavaScript linting files. Default: false", | ||
aliases: "-j" | ||
# --ruby-linters | ||
class_option :ruby_linters, | ||
type: :boolean, | ||
default: false, | ||
desc: "Install ruby linting files, tasks, and configs", | ||
desc: "Install ruby linting files, tasks, and configs. Default: false", | ||
aliases: "-L" | ||
# --ruby-linters | ||
class_option :heroku_deployment, | ||
type: :boolean, | ||
default: false, | ||
desc: "Install files necessary for deploying to Heroku", | ||
desc: "Install files necessary for deploying to Heroku. Default: false", | ||
aliases: "-H" | ||
|
||
# --skip-bootstrap | ||
class_option :skip_bootstrap, | ||
type: :boolean, | ||
default: false, | ||
desc: "Skip integrating Bootstrap and don't initialize files and regarding configs", | ||
desc: "Skip integrating Bootstrap and don't initialize files and regarding configs. Default: false", | ||
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. @justin808 I feel very strongly that this is adding confusion, not removing it. Now when we say the default is 'false,' is |
||
aliases: "-b" | ||
|
||
def run_generators # rubocop:disable Metrics/CyclomaticComplexity | ||
|
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.
"setup involved when
withintegrating"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.
"Defaults for options are such that the default is for the flag to be off."
@justin808 I still think the defaults thing is self-explanatory from running the rake descriptions. I'd at least change this sentence as it's super confusing.
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.
@justin808 Also, a lot of this information, both the generators and linters information, is completely redundant and is already inside of the Readme. We should not have documentation for the same exact thing twice. The last thing we need to do is add more stuff to read. Let's either use the README version or this version, but not both? Then we delete what's in the README so that we only have to update this info in a single place from now on.