-
-
Notifications
You must be signed in to change notification settings - Fork 530
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 suspenders:development:environment
generator
#1149
Add suspenders:development:environment
generator
#1149
Conversation
eab6dbe
to
54d9428
Compare
I feel like it makes sense to create a
I think this should be its own separate generator, since there's an opportunity to improve it. That would also be the place to handle
It looks like that was introduced in 2015 in 3a06edf and might be in reference to this gem. I think we should stick with the Rails defaults and not modify
I don't think we need to configure the generators. If we still feel strongly about this, that could be a responsibility of the
Yes, this is on purpose. I don't think it's explicitly documented anywhere, but Rails seems to default to yarn. For example, if you install cssbundling-rails, it will install the dependencies with yarn. Additionally, Rails ships with a yarn Rake task |
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.
I love how lean this is! I think we should take this opportunity to review all configuration options.
For example, I think there's value in enabling config.action_view.annotate_rendered_view_with_filenames.
Do we want to enable config.active_record.strict_loading_by_default?
def raise_on_missing_translations_in_test | ||
uncomment_lines("config/environments/test.rb", "config.i18n.raise_on_missing_translations = true") | ||
end |
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.
I recognize this is an open question in the pull request, but want to note that we're modifying files in the test
environment even though this generator is called DevelopmentEnvironmentGenerator
.
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.
We might want to add this and other tweaks to the test environment, so I will remove this from the development environment generator.
Into it.
Is this a development tool? This seems like something that will change semantics, and I wouldn't want it to be different in different environments. |
Yes. We have it enabled on in |
Oh I see how it works now. Wow that thing is under-documented. Yup that makes sense to me. |
In Suspenders 2, we had namespaced generators such as |
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.
From a test-driven development perspective, the change to set config.action_dispatch.show_exceptions = :rescuable
by default (introduced in rails/rails#45867 had had a largely negative impact on the feedback from Controller and System level test failures.
I'm advocating that Suspenders set config.action_dispatch.show_exceptions = :none
so that tests fails with callstack information instead of HTML error pages.
Based on a recent client feature, I think there's value in enabling config.active_model.i18n_customize_full_message. I've opened rails/rails#50406 to make this the default. |
suspenders:development_environment
generatorsuspenders:development:environment
generator
32e96e7
to
e17e574
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Noting we might want to enable |
Creates generator to configuration the production environment. For now, this means [requiring a master key][master key]. Drops configuration for [asset_host][], since that should be an infrastructure decision. In an effort to distinguish between the development environment configuration in #1149, we use the `production` namespace. [master key]: https://guides.rubyonrails.org/configuring.html#config-require-master-key [asset_host]: https://guides.rubyonrails.org/configuring.html#config-asset-host
e17e574
to
1711f5b
Compare
Creates generator to configuration the production environment. For now, this means [requiring a master key][master key]. Drops configuration for [asset_host][], since that should be an infrastructure decision. In an effort to distinguish between the development environment configuration in #1149, we use the `production` namespace. [master key]: https://guides.rubyonrails.org/configuring.html#config-require-master-key [asset_host]: https://guides.rubyonrails.org/configuring.html#config-asset-host
Configures test environment. This differs from #1156 in that this commit is concerned with configuration, where that commit was concerned with generating a holistic test suite. It's also possible to run each generator independently, and the two should not rely on one another. Disables [action_dispatch.show_exceptions][] in an effort to [improve failure output][comment]. Enables [raise_on_missing_translations][] to keep parity with the same setting in development #1149 [action_dispatch.show_exceptions]: https://edgeguides.rubyonrails.org/configuring.html#config-action-dispatch-show-exceptions [comment]: #1149 (comment) [raise_on_missing_translations]: https://guides.rubyonrails.org/configuring.html#config-i18n-raise-on-missing-translations
Creates generator to configuration the production environment. For now, this means [requiring a master key][master key]. Drops configuration for [asset_host][], since that should be an infrastructure decision. In an effort to distinguish between the development environment configuration in #1149, we use the `production` namespace. [master key]: https://guides.rubyonrails.org/configuring.html#config-require-master-key [asset_host]: https://guides.rubyonrails.org/configuring.html#config-asset-host
Configures test environment. This differs from #1156 in that this commit is concerned with configuration, where that commit was concerned with generating a holistic test suite. It's also possible to run each generator independently, and the two should not rely on one another. Disables [action_dispatch.show_exceptions][] in an effort to [improve failure output][comment]. Enables [raise_on_missing_translations][] to keep parity with the same setting in development #1149 [action_dispatch.show_exceptions]: https://edgeguides.rubyonrails.org/configuring.html#config-action-dispatch-show-exceptions [comment]: #1149 (comment) [raise_on_missing_translations]: https://guides.rubyonrails.org/configuring.html#config-i18n-raise-on-missing-translations
96180f8
to
12ed7db
Compare
12ed7db
to
4b8894d
Compare
Creates generator to configuration the production environment. For now, this means [requiring a master key][master key]. Drops configuration for [asset_host][], since that should be an infrastructure decision. In an effort to distinguish between the development environment configuration in #1149, we use the `production` namespace. [master key]: https://guides.rubyonrails.org/configuring.html#config-require-master-key [asset_host]: https://guides.rubyonrails.org/configuring.html#config-asset-host Co-authored-by: Steve Polito <stevepolito@hey.com>
Configures test environment. This differs from #1156 in that this commit is concerned with configuration, where that commit was concerned with generating a holistic test suite. It's also possible to run each generator independently, and the two should not rely on one another. Disables [action_dispatch.show_exceptions][] in an effort to [improve failure output][comment]. Enables [raise_on_missing_translations][] to keep parity with the same setting in development #1149 [action_dispatch.show_exceptions]: https://edgeguides.rubyonrails.org/configuring.html#config-action-dispatch-show-exceptions [comment]: #1149 (comment) [raise_on_missing_translations]: https://guides.rubyonrails.org/configuring.html#config-i18n-raise-on-missing-translations
Configures test environment. This differs from #1156 in that this commit is concerned with configuration, where that commit was concerned with generating a holistic test suite. It's also possible to run each generator independently, and the two should not rely on one another. Disables [action_dispatch.show_exceptions][] in an effort to [improve failure output][comment]. Enables [raise_on_missing_translations][] to keep parity with the same setting in development #1149 [action_dispatch.show_exceptions]: https://edgeguides.rubyonrails.org/configuring.html#config-action-dispatch-show-exceptions [comment]: #1149 (comment) [raise_on_missing_translations]: https://guides.rubyonrails.org/configuring.html#config-i18n-raise-on-missing-translations
4b8894d
to
50f2a8d
Compare
Creates generator to configure the development environment. Keeps parity with Rails as much as possible in an effort to avoid drift with Rails standards. It's possible a future release of Rails will alleviate us from having to do the following: - enable `active_model.i18n_customize_full_message` which is being addressed in [#50406][] - enable `active_record.query_log_tags_enabled` which is being addressed in [#51342][] [#50406]: rails/rails#50406 [#51342]: rails/rails#51342
50f2a8d
to
cca73b5
Compare
Creates generator to configuration the production environment. For now, this means [requiring a master key][master key]. Drops configuration for [asset_host][], since that should be an infrastructure decision. In an effort to distinguish between the development environment configuration in #1149, we use the `production` namespace. [master key]: https://guides.rubyonrails.org/configuring.html#config-require-master-key [asset_host]: https://guides.rubyonrails.org/configuring.html#config-asset-host Co-authored-by: Steve Polito <stevepolito@hey.com>
Configures test environment. This differs from #1156 in that this commit is concerned with configuration, where that commit was concerned with generating a holistic test suite. It's also possible to run each generator independently, and the two should not rely on one another. Disables [action_dispatch.show_exceptions][] in an effort to [improve failure output][comment]. Enables [raise_on_missing_translations][] to keep parity with the same setting in development #1149 [action_dispatch.show_exceptions]: https://edgeguides.rubyonrails.org/configuring.html#config-action-dispatch-show-exceptions [comment]: #1149 (comment) [raise_on_missing_translations]: https://guides.rubyonrails.org/configuring.html#config-i18n-raise-on-missing-translations
Creates generator to configure the development environment. Keeps parity with Rails as much as possible in an effort to avoid drift with Rails standards. It's possible a future release of Rails will alleviate us from having to do the following: - enable `active_model.i18n_customize_full_message` which is being addressed in [#50406][] - enable `active_record.query_log_tags_enabled` which is being addressed in [#51342][] [#50406]: rails/rails#50406 [#51342]: rails/rails#51342 Co-authored-by: Steve Polito <stevepolito@hey.com>
Creates generator to configure the development environment. Keeps parity
with Rails as much as possible in an effort to avoid drift with Rails
standards.
It's possible a future release of Rails will alleviate us from having to
do the following:
active_model.i18n_customize_full_message
which is beingaddressed in #50406
active_record.query_log_tags_enabled
which is being addressedin #51342