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

Instructions to wrap initializer in development check #12

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

bpurinton
Copy link
Contributor

@bpurinton bpurinton commented Oct 18, 2024

I noticed that for our Rails projects which all have this line in the bin/setup script:

bundle exec rake db:create RAILS_ENV=test

setup fails with this error when the config/initializers/dev_toolbar.rb is not wrapped in a development environment check:

== Creating test database ==
rake aborted!
NameError: uninitialized constant DevToolbar (NameError)

DevToolbar.configure do |config|
^^^^^^^^^^
/Users/ben/Desktop/firstdraft-github/orgs/appdev-projects/phase1/rails-rps/config/initializers/dev_toolbar.rb:1:in `<main>'
/Users/ben/Desktop/firstdraft-github/orgs/appdev-projects/phase1/rails-rps/config/environment.rb:5:in `<main>'
<internal:/Users/ben/.rbenv/versions/3.2.1/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
<internal:/Users/ben/.rbenv/versions/3.2.1/lib/ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:37:in `require'
/Users/ben/.rbenv/versions/3.2.1/bin/bundle:25:in `load'
/Users/ben/.rbenv/versions/3.2.1/bin/bundle:25:in `<main>'
Tasks: TOP => db:create => db:load_config => environment
(See full trace by running task with --trace)

== Command ["bundle exec rake db:create RAILS_ENV=test"] failed ==

That surprised me given this code in the gem:

if Rails.env.development? && headers["Content-Type"]&.include?("text/html")


Perhaps there is a better solution than just updating the README to point out this requirement, however, on #8, there is already planned work to create a generator for the initializer. During that work, we can investigate this more deeply, but for now, since we need this working ASAP for class, I think we just need this instruction in the README and to have all the initializers use this slight hack.

Since all initializers will be uniform across our Rails projects and managed with the project-syncing tool, it will be easy to fix this later on across the projects that are using the dev_toolbar gem.


Important

Wraps DevToolbar.configure in a development check to prevent test database creation errors, updating README.md accordingly.

  • Behavior:
    • Wraps DevToolbar.configure in config/initializers/dev_toolbar.rb with if Rails.env.development? to prevent errors during test database creation.
  • Documentation:
    • Updates README.md to include the development environment check in the configuration example.

This description was created by Ellipsis for f47a9f2. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to f47a9f2 in 8 seconds

More details
  • Looked at 28 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. README.md:26
  • Draft comment:
    The addition of the Rails.env.development? check is a good practice to ensure that the DevToolbar is only configured in the development environment, preventing errors in other environments.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The PR correctly wraps the initializer in a development environment check, which is necessary to prevent errors during test database creation. This change is consistent with the PR description and addresses the issue effectively.

Workflow ID: wflow_GHWKc5SuAkqbz0U4


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@bpurinton bpurinton merged commit 33be3d3 into main Oct 21, 2024
@bpurinton bpurinton deleted the bp-wrap-initializer-in-dev-check branch October 21, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant