8000 Revert "Ensure application gets reloaded only once" · activeadmin/activeadmin@391ef6a · GitHub
[go: up one dir, main page]
Skip to content

Commit 391ef6a

Browse files
deivid-rodriguezdeivid-rodriguez
committed
Revert "Ensure application gets reloaded only once"
This reverts commit 21b6138, because it created a performance regression in development when reloading code.
1 parent 97e48ec commit 391ef6a

File tree

9 files changed

+18
-36
lines changed

9 files changed

+18
-36
lines changed

‎.circleci/config.yml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,6 @@ version: 2.1
9494
command: |
9595
COVERAGE=true PARALLEL_TEST_PROCESSORS=4 bin/parallel_rspec spec/
9696
COVERAGE=true RSPEC_FILESYSTEM_CHANGES=true bin/rspec
97-
COVERAGE=true CLASS_RELOADING=true bin/rspec
9897
9998
.run_features: &run_features
10099
run:

‎.rspec

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
--format <%= ENV['CI'] ? 'documentation' : 'progress' %>
22
--require spec_helper
33
<%= "--require #{__dir__}/spec/support/simplecov_changes_env.rb --tag changes_filesystem" if ENV['RSPEC_FILESYSTEM_CHANGES'] %>
4-
<%= "--require #{__dir__}/spec/support/simplecov_reload_env.rb --tag requires_reloading" if ENV['CLASS_RELOADING'] %>

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
### Bug Fixes
1313

1414
* Fix pundit policy retrieving for static pages when the pundit namespace is :active_admin. [#5777] by [@kwent]
15+
* Revert [21b6138f] from [#5740] since they actually caused the performance in development to regress. [#5801] by [@deivid-rodriguez]
1516

1617
## 2.1.0 [☰](https://github.com/activeadmin/activeadmin/compare/v2.0.0..v2.1.0)
1718

@@ -366,6 +367,8 @@ Please check [0-6-stable] for previous changes.
366367
[#5043]: https://github.com/activeadmin/activeadmin/issues/5043
367368
[#5198]: https://github.com/activeadmin/activeadmin/issues/5198
368369

370+
[21b6138f]: https://github.com/activeadmin/activeadmin/pull/5740/commits/21b6138fdcf58cd54c3f1d3f60cb1127b174b40f
371+
369372
[#3091]: https://github.com/activeadmin/activeadmin/pull/3091
370373
[#3435]: https://github.com/activeadmin/activeadmin/pull/3435
371374
[#4477]: https://github.com/activeadmin/activeadmin/pull/4477
@@ -468,6 +471,7 @@ Please check [0-6-stable] for previous changes.
468471
[#5758]: https://github.com/activeadmin/activeadmin/pull/5758
469472
[#5777]: https://github.com/activeadmin/activeadmin/pull/5777
470473
[#5794]: https://github.com/activeadmin/activeadmin/pull/5794
474+
[#5801]: https://github.com/activeadmin/activeadmin/pull/5801
471475

472476
[@5t111111]: https://github.com/5t111111
473477
[@aarek]: https://github.com/aarek

‎lib/active_admin/application.rb

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,17 @@ def remove_active_admin_load_paths_from_rails_autoload_and_eager_load
187187
# regenerate the routes as well.
188188
def attach_reloader
189189
Rails.application.config.after_initialize do |app|
190-
ActiveSupport::Reloader.after_class_unload do
191-
ActiveAdmin.application.unload!
190+
unload_active_admin = -> { ActiveAdmin.application.unload! }
191+
192+
if app.config.reload_classes_only_on_change
193+
# Rails is about to unload all the app files (e.g. models), so we
194+
# should first unload the classes generated by Active Admin, otherwise
195+
# they will contain references to the stale (unloaded) classes.
196+
ActiveSupport::Reloader.to_prepare(prepend: true, &unload_active_admin)
197+
else
198+
# If the user has configured the app to always reload app files after
199+
# each request, so we should unload the generated classes too.
200+
ActiveSupport::Reloader.to_complete(&unload_active_admin)
192201
end
193202

194203
admin_dirs = {}

‎spec/integration/rails_integration_spec.rb

Lines changed: 0 additions & 13 deletions
This file was deleted.

‎spec/spec_helper.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
RSpec.configure do |config|
44
config.disable_monkey_patching!
55
config.filter_run focus: true
6-
config.filter_run_excluding changes_filesystem: true,
7-
requires_reloading: true
6+
config.filter_run_excluding changes_filesystem: true
87
config.run_all_when_everything_filtered = true
98
config.color = true
109
config.order = :random

‎spec/support/simplecov_reload_env.rb

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎spec/unit/application_spec.rb

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,7 @@
99
end
1010

1111
describe "#prepare" do
12-
before do
13-
%i[run complete prepare class_unload].each do |event|
14-
ActiveSupport::Reloader.reset_callbacks(event)
15-
end
16-
application.prepare!
17-
end
12+
before { application.prepare! }
1813

1914
it "should remove app/admin from the autoload paths" do
2015
expect(ActiveSupport::Dependencies.autoload_paths).to_not include(Rails.root.join("app/admin"))

‎tasks/test.rake

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ task spec: :"spec:all"
1212

1313
namespace :spec do
1414
desc "Run all specs"
15-
task all: [:regular, :filesystem_changes, :reloading]
15+
task all: [:regular, :filesystem_changes]
1616

1717
desc "Run the standard specs in parallel"
1818
task :regular do
@@ -23,11 +23,6 @@ namespace :spec do
2323
task :filesystem_changes do
2424
sh({ "RSPEC_FILESYSTEM_CHANGES" => "true" }, "bin/rspec")
2525
end
26-
27-
desc "Run the specs that require reloading"
28-
task :reloading do
29-
sh({ "CLASS_RELOADING" => "true" }, "bin/rspec")
30-
end
3126
end
3227

3328
desc "Run the cucumber scenarios in parallel"

0 commit comments

Comments
 (0)
0