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

Autoload sotrage adapters #555

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gmcgibbon
Copy link
Contributor

Autoloads storage adapters because it is unnecessary to load storage
adapters you won't use. Autoload lets us defer loading the file until it
is referenced, so that we can require storage dependencies normally and
expect them to load when called.

Depends on #551. I can't make a PR to this repo if my merge target is on my fork, so this will have to do for now.

This file contains Rack::MiniProfiler, not Rack::MiniProfiler::Profiler.
- Make module files require their nested contents
- Move library requires to the files that use them
- Remove require for timeout because it isn't used
- Remove require for thread because it is required by default

This patch also allows for easier transition to autoloading should we
choose to use it later.
Autoloads storage adapters because it is unnecessary to load storage
adapters you won't use. Autoload lets us defer loading the file until it
is referenced, so that we can require storage dependencies normally and
expect them to load when called.
@codecov-commenter
Copy link

codecov-commenter commented Feb 8, 2023

Codecov Report

Merging #555 (e919aba) into master (edbcad3) will increase coverage by 0.02%.
The diff coverage is 96.55%.

❗ Current head e919aba differs from pull request most recent head 4026a0f. Consider uploading reports for the commit 4026a0f to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master     #555      +/-   ##
==========================================
+ Coverage   87.39%   87.42%   +0.02%     
==========================================
  Files          18       20       +2     
  Lines        1269     1272       +3     
==========================================
+ Hits         1109     1112       +3     
  Misses        160      160              
Impacted Files Coverage Δ
lib/mini_profiler.rb 83.85% <92.30%> (ø)
lib/mini_profiler/storage.rb 100.00% <100.00%> (ø)
lib/mini_profiler/timer_struct.rb 100.00% <100.00%> (ø)
lib/mini_profiler/timer_struct/base.rb 100.00% <100.00%> (ø)
lib/mini_profiler/timer_struct/sql.rb 85.36% <100.00%> (+0.36%) ⬆️
lib/rack-mini-profiler.rb 85.71% <100.00%> (-10.84%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@suryanarayanan035
Copy link

The code changes Looks good to me. We just need to sync the autoload_storage_adapaters with the latest master and resolve the conflict. After @gmcgibbon is done with the mentioned action items, we can merge this code @nateberkopec .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants