From 4f1295129b22f435641c4d63a15bfd4395db98ca Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Tue, 16 Oct 2018 01:42:15 +0300 Subject: Unify changelog entries related to `database` option of Rails generators [ci skip] `migrations_paths` option was added to migration generator, with changelog entry, in #33760. Also `migrations_paths` option was added to model generator, with changelog entry, in #33994. Then `migrations_paths` was renamed to `database` and aliased as `db` in #34021, and was added new changelog entry. I think we should edit existed changelog entries instead adding new about changing the name of the option from `migrations_paths` to `database` since Rails 6.0 hasn't been released yet, and since It might confuse readers of the changelog file in case if they've read changelog enty about adding `migrations_paths` option but haven't read the entry about change the name of that option to `database`. @eileencodes, @gmcgibbon, @rafaelfranca Does it make sense? --- railties/CHANGELOG.md | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) (limited to 'railties') diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 1a415449d1..f82b080f12 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -11,34 +11,18 @@ *DHH*, *Lachlan Sylvester* -* Refactors `migrations_paths` command option in generators - to `database` (aliased as `db`). Now, the migrations paths - will be read from the specified database configuration in the - current environment. +* Add `database` (aliased as `db`) option to model generator to allow + setting the database. This is useful for applications that use + multiple databases and put migrations per database in their own directories. ``` - bin/rails g model Chair brand:string --database=kingston - invoke active_record - create db/kingston_migrate/20180830151055_create_chairs.rb - ``` - - `--database` can be used with the migration, model, and scaffold generators. - - *Gannon McGibbon* - -* Adds an option to the model generator to allow setting the - migrations paths for that migration. This is useful for - applications that use multiple databases and put migrations - per database in their own directories. - - ``` - bin/rails g model Room capacity:integer --migrations-paths=db/kingston_migrate + bin/rails g model Room capacity:integer --database=kingston invoke active_record create db/kingston_migrate/20180830151055_create_rooms.rb ``` Because rails scaffolding uses the model generator, you can - also specify migrations paths with the scaffold generator. + also specify a database with the scaffold generator. *Gannon McGibbon* @@ -66,15 +50,15 @@ *Yoshiyuki Kinjo* -* Add `--migrations_paths` option to migration generator. +* Add `database` (aliased as `db`) option to migration generator. If you're using multiple databases and have a folder for each database for migrations (ex db/migrate and db/new_db_migrate) you can now pass the - `--migrations_paths` option to the generator to make sure the the migration + `--database` option to the generator to make sure the the migration is inserted into the correct folder. ``` - rails g migration CreateHouses --migrations_paths=db/kingston_migrate + rails g migration CreateHouses --database=kingston invoke active_record create db/kingston_migrate/20180830151055_create_houses.rb ``` -- cgit v1.2.3 From 60e6269598b9ac01fc73517e03e786a5470772cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= Date: Wed, 17 Oct 2018 12:51:23 -0300 Subject: Fix generated Gemfile missing gems on jruby --- railties/lib/rails/generators/rails/app/templates/Gemfile.tt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'railties') diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt index 1567333023..fb264935bd 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt @@ -45,6 +45,7 @@ group :development, :test do gem 'byebug', platforms: [:mri, :mingw, :x64_mingw] end +<% end -%> group :development do <%- unless options.api? -%> # Access an interactive console on exception pages or by calling 'console' anywhere in the code. @@ -75,7 +76,6 @@ group :test do gem 'chromedriver-helper' end <%- end -%> -<% end -%> # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] -- cgit v1.2.3 From 86a12b6f2932ba1a3b5f0417688ffe34ea6bab25 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Fri, 19 Oct 2018 17:56:09 +0900 Subject: Avoid running bundler on tests that don't need it If the dev option is specified, Gemfile contains gem which specifies GitHub. This will take time to execute, so should avoid it in unnecessary tests. --- railties/test/generators/app_generator_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'railties') diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index be97abfa52..e47d180809 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -710,7 +710,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_web_console_with_dev_option - run_generator [destination_root, "--dev"] + run_generator [destination_root, "--dev", "--skip-bundle"] assert_file "Gemfile" do |content| assert_match(/gem 'web-console',\s+github: 'rails\/web-console'/, content) @@ -780,7 +780,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_spring_with_dev_option - run_generator [destination_root, "--dev"] + run_generator [destination_root, "--dev", "--skip-bundle"] assert_no_gem "spring" end @@ -878,7 +878,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_bootsnap_with_dev_option - run_generator [destination_root, "--dev"] + run_generator [destination_root, "--dev", "--skip-bundle"] assert_no_gem "bootsnap" assert_file "config/boot.rb" do |content| -- cgit v1.2.3 From 0f7655c32565fe263c2401c98573b4cce49ecb4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francesco=20Rodr=C3=ADguez?= Date: Fri, 19 Oct 2018 15:56:27 +0200 Subject: Remove unnecessary escape character --- .../generators/rails/app/templates/app/javascript/channels/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'railties') diff --git a/railties/lib/rails/generators/rails/app/templates/app/javascript/channels/index.js b/railties/lib/rails/generators/rails/app/templates/app/javascript/channels/index.js index 5da1ce2dce..0cfcf74919 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/javascript/channels/index.js +++ b/railties/lib/rails/generators/rails/app/templates/app/javascript/channels/index.js @@ -1,5 +1,5 @@ -// Load all the channels within this directory and all subdirectories. +// Load all the channels within this directory and all subdirectories. // Channel files must be named *_channel.js. -const channels = require.context('.', true, /\_channel\.js$/) +const channels = require.context('.', true, /_channel\.js$/) channels.keys().forEach(channels) -- cgit v1.2.3 From 931120d37d8f204f0e8c2aabbe4caa68347ee7b9 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Sat, 20 Oct 2018 14:16:29 +0900 Subject: Avoid running `webpacker:install` on tests that don't need it If use `run_generator` to run the generator, `--skip-webpack-install` is specified automatically. https://github.com/rails/rails/blob/3101a4136bd62787e252d2658eee23001036fa0f/railties/lib/rails/generators/testing/behaviour.rb#L71 However, when executing the generator independently (for example, to use stub), `webpacker:install` was executed. Since this includes `yarn install`, it should be avoided in unnecessary testing. --- railties/test/generators/app_generator_test.rb | 12 ++++++++---- railties/test/generators/shared_generator_tests.rb | 4 ++-- 2 files changed, 10 insertions(+), 6 deletions(-) (limited to 'railties') diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index e47d180809..e282c82faf 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -104,7 +104,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_skip_bundle - assert_not_called(generator([destination_root], skip_bundle: true), :bundle_command) do + assert_not_called(generator([destination_root], skip_bundle: true, skip_webpack_install: true), :bundle_command) do quietly { generator.invoke_all } # skip_bundle is only about running bundle install, ensure the Gemfile is still # generated. @@ -728,13 +728,13 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_generation_runs_bundle_install - generator([destination_root], {}) + generator([destination_root], skip_webpack_install: true) assert_bundler_command_called("install") end def test_dev_option - generator([destination_root], dev: true) + generator([destination_root], dev: true, skip_webpack_install: true) assert_bundler_command_called("install") rails_path = File.expand_path("../../..", Rails.root) @@ -742,7 +742,7 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_edge_option - generator([destination_root], edge: true) + generator([destination_root], edge: true, skip_webpack_install: true) assert_bundler_command_called("install") assert_file "Gemfile", %r{^gem\s+["']rails["'],\s+github:\s+["']#{Regexp.escape("rails/rails")}["']$} @@ -754,12 +754,16 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_bundler_binstub + generator([destination_root], skip_webpack_install: true) + assert_bundler_command_called("binstubs bundler") end def test_spring_binstubs jruby_skip "spring doesn't run on JRuby" + generator([destination_root], skip_webpack_install: true) + assert_bundler_command_called("exec spring binstub --all") end diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index fc654e867b..d61545a7b5 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -91,7 +91,7 @@ module SharedGeneratorTests template end - generator([destination_root], template: path).stub(:open, check_open, template) do + generator([destination_root], template: path, skip_webpack_install: true).stub(:open, check_open, template) do generator.stub :bundle_command, nil do quietly { assert_match(/It works!/, capture(:stdout) { generator.invoke_all }) } end @@ -99,7 +99,7 @@ module SharedGeneratorTests end def test_skip_gemfile - assert_not_called(generator([destination_root], skip_gemfile: true), :bundle_command) do + assert_not_called(generator([destination_root], skip_gemfile: true, skip_webpack_install: true), :bundle_command) do quietly { generator.invoke_all } assert_no_file "Gemfile" end -- cgit v1.2.3 From b004e767e03f11eb7e13828a5efbe030eb861cb0 Mon Sep 17 00:00:00 2001 From: Bogdan Date: Sun, 21 Oct 2018 20:29:58 +0300 Subject: Correct some tests related to changes in #33079 (#34272) * Fix assertions of auto-generated ActiveStorage JS Since #33079 * Correct message on the `assert_equal` failure Related to #33079 * Test ActionCable's js files This commit adds `app/javascript/channels/consumer.js`, and `app/javascript/channels/index.js` to `DEFAULT_APP_FILES` in order to assert their existance in `test_skeleton_is_created`. Related to #33079 * Assert no match `javascript_pack_tag` in `application.html.erb` Since #33079 `rails new` generates `application.html.erb` file with `javascript_pack_tag` instead of `javascript_include_tag`. Note that there some tests that asserting no matching `javascript_include_tag` in the `application.html.erb` file for newly generated rails plugins. It is related to #34009 and shouldn't be changed right now. --- railties/test/generators/app_generator_test.rb | 6 ++++-- railties/test/generators/shared_generator_tests.rb | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) (limited to 'railties') diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index e282c82faf..f5356a358f 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -15,6 +15,8 @@ DEFAULT_APP_FILES = %w( app/assets/images app/javascript app/javascript/channels + app/javascript/channels/consumer.js + app/javascript/channels/index.js app/javascript/packs/application.js app/assets/stylesheets app/assets/stylesheets/application.css @@ -608,7 +610,7 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "app/views/layouts/application.html.erb" do |contents| assert_match(/stylesheet_link_tag\s+'application', media: 'all' %>/, contents) - assert_no_match(/javascript_include_tag\s+'application' \%>/, contents) + assert_no_match(/javascript_pack_tag\s+'application'/, contents) end end @@ -794,7 +796,7 @@ class AppGeneratorTest < Rails::Generators::TestCase @called ||= 0 if command == "webpacker:install" @called += 1 - assert_equal 0, @called, "webpacker:install expected not to be called once, but was called #{@called} times." + assert_equal 0, @called, "webpacker:install expected not to be called, but was called #{@called} times." end end diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index d61545a7b5..398466aa22 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -231,7 +231,7 @@ module SharedGeneratorTests assert_file "#{application_path}/config/application.rb", /#\s+require\s+["']active_storage\/engine["']/ assert_file "#{application_path}/app/javascript/packs/application.js" do |content| - assert_no_match(/^\/\/= require activestorage/, content) + assert_no_match(/activestorage/, content) end assert_file "#{application_path}/config/environments/development.rb" do |content| @@ -261,7 +261,7 @@ module SharedGeneratorTests assert_file "#{application_path}/config/application.rb", /#\s+require\s+["']active_storage\/engine["']/ assert_file "#{application_path}/app/javascript/packs/application.js" do |content| - assert_no_match(/^import * as ActiveStorage from "activestorage"\nActiveStorage.start()/, content) + assert_no_match(/^import * as ActiveStorage from "activestorage"\nActiveStorage.start\(\)/, content) end assert_file "#{application_path}/config/environments/development.rb" do |content| -- cgit v1.2.3 From 9629354abbb5e96142834497f80d267e96536ced Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Sun, 21 Oct 2018 22:40:01 +0300 Subject: Remove yarn's files from `.gitignore` template for new rails app Webpacker already does it, see https://github.com/rails/webpacker/blob/895d2cfc15eda2edae9e667c642a02523d958f53/lib/install/template.rb#L25-L33 I also opened PR https://github.com/rails/webpacker/pull/1765 in order to make it add `/yarn-error.log` file too. --- railties/lib/rails/generators/rails/app/templates/gitignore.tt | 7 +------ railties/test/generators/shared_generator_tests.rb | 10 ---------- 2 files changed, 1 insertion(+), 16 deletions(-) (limited to 'railties') diff --git a/railties/lib/rails/generators/rails/app/templates/gitignore.tt b/railties/lib/rails/generators/rails/app/templates/gitignore.tt index 4e114fb1d9..38c3ab1319 100644 --- a/railties/lib/rails/generators/rails/app/templates/gitignore.tt +++ b/railties/lib/rails/generators/rails/app/templates/gitignore.tt @@ -27,14 +27,9 @@ <% if keeps? -%> !/storage/.keep <% end -%> -<% end -%> - -<% unless options.skip_yarn? -%> -/node_modules -/yarn-error.log - <% end -%> <% unless options.api? -%> + /public/assets <% end -%> .byebug_history diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index 398466aa22..9b980bd52b 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -339,11 +339,6 @@ module SharedGeneratorTests run_generator assert_file "#{application_path}/package.json", /dependencies/ assert_file "#{application_path}/config/initializers/assets.rb", /node_modules/ - - assert_file ".gitignore" do |content| - assert_match(/node_modules/, content) - assert_match(/yarn-error\.log/, content) - end end def test_generator_for_yarn_skipped @@ -354,10 +349,5 @@ module SharedGeneratorTests assert_file "#{application_path}/config/initializers/assets.rb" do |content| assert_no_match(/node_modules/, content) end - - assert_file ".gitignore" do |content| - assert_no_match(/node_modules/, content) - assert_no_match(/yarn-error\.log/, content) - end end end -- cgit v1.2.3 From 7093ee8ec5250545ba904a2cb1fcd67c5138cc3b Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Mon, 22 Oct 2018 00:00:32 +0300 Subject: Remove `:javascript` from `Rails::PluginBuilder::PASSTHROUGH_OPTIONS` `--javascript` option was removed by 42198064c35ff3b701496309f90df2abc229efbe --- railties/lib/rails/generators/rails/plugin/plugin_generator.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'railties') diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index a018a98c53..532278f215 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -88,7 +88,7 @@ task default: :test PASSTHROUGH_OPTIONS = [ :skip_active_record, :skip_active_storage, :skip_action_mailer, :skip_javascript, :skip_action_cable, :skip_sprockets, :database, - :javascript, :skip_yarn, :api, :quiet, :pretend, :skip + :skip_yarn, :api, :quiet, :pretend, :skip ] def generate_test_dummy(force = false) -- cgit v1.2.3 From 24367edbe6dae8fa1878423254d445177540e739 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Mon, 22 Oct 2018 00:12:15 +0300 Subject: Remove `javascripts` and `javascript_engine` options for generators It is unused since #33079 --- railties/lib/rails/generators.rb | 2 -- 1 file changed, 2 deletions(-) (limited to 'railties') diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index cd8f16e247..5e8cebc50a 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -54,8 +54,6 @@ module Rails force_plural: false, helper: true, integration_tool: nil, - javascripts: true, - javascript_engine: :js, orm: false, resource_controller: :controller, resource_route: true, -- cgit v1.2.3 From 2593ee3f49d03f70799cc77e64f4c27954c908c8 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Mon, 22 Oct 2018 01:33:10 +0300 Subject: Remove extra call `remove_file` on `rails new` with `--skip_action_cable` There is no need to remove this file since the line below removes entire directory in which that file is placed. --- railties/lib/rails/generators/rails/app/app_generator.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'railties') diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index b118ea989b..1c7cf1a951 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -438,7 +438,6 @@ module Rails def delete_action_cable_files_skipping_action_cable if options[:skip_action_cable] - remove_file "app/javascript/channels/consumer.js" remove_dir "app/javascript/channels" remove_dir "app/channels" end -- cgit v1.2.3