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

Update the "Configuring Rails Applications" guide [ci skip] #34855

Merged
merged 1 commit into from
Jan 3, 2019

Conversation

bogdanvlviv
Copy link
Contributor

@bogdanvlviv bogdanvlviv commented Jan 3, 2019

Add section "Results of load_defaults" to the guide.

We should talk about the default values of configurations in the context of a newly generated rails app on the specific version. New rails apps are generated with config.load_defaults 6.0 in config/application.rb, so `config.active_job.return_false_on_aborted_enqueue` is set to `true`. Related to https://github.com/rails/rails/commit/884310fdd031ed8121944f9ea07c8b7723c4e6b6#r31819118, https://github.com//pull/33172#discussion_r196960655

Similar case c8a22bb

Also, I think it isn't a good idea to talk about previous Rails versions
in the current guide since we use versions for guides and if users want to
know how something works on a previous version of Rails, they should read
the guides of the specific version.

@rafaelfranca
Copy link
Member

Sorry but I disagree. We should talk about the default value of the configuration when no railtie code is applied. So the current documentation is correct. If we tell that the default is true for config.action_controller.default_protect_from_forgery and the user see that config being set to true in the application they will think it is safe to remove when it is not.

We could talk which configs are applied when the version defaults are set but each individual config should have its default value not depending on the load_defaults call.

Let's change this PR to specify the defaults in each rails version up to 6.0.

@bogdanvlviv
Copy link
Contributor Author

Let's change this PR to specify the defaults in each rails version up to 6.0.

@rafaelfranca Do you want me to apply this change only, right?:

diff --git a/guides/source/configuring.md b/guides/source/configuring.md
index 3b21197ae4..6bb5a1e03b 100644
--- a/guides/source/configuring.md
+++ b/guides/source/configuring.md
@@ -805,7 +805,7 @@ There are a few configuration options available in Active Support:

 * `config.active_job.custom_serializers` allows to set custom argument serializers. Defaults to `[]`.

-* `config.active_job.return_false_on_aborted_enqueue` change the return value of `#enqueue` to false instead of the job instance when the enqueuing is aborted. Defaults to `false`.
+* `config.active_job.return_false_on_aborted_enqueue` change the return value of `#enqueue` to false instead of the job instance when the enqueuing is aborted. Defaults to `false` up to Rails 6.0.

@rafaelfranca
Copy link
Member

I'd prefer to remove those versions specifics from the config and add a new section like:

# Results of `load_defaults`

## With '6.0':

* `config.active_job.return_false_on_aborted_enqueue`: false
* `config.foo.bar`: true

@bogdanvlviv
Copy link
Contributor Author

Sorry but I disagree. We should talk about the default value of the configuration when no railtie code is applied. So the current documentation is correct. If we tell that the default is true for config.action_controller.default_protect_from_forgery and the user see that config being set to true in the application they will think it is safe to remove when it is not.

Thanks for the explanation, that’s a point.

I'd prefer to remove those versions specifics from the config and add a new section like:

Good idea. I just amended this PR.

Add section "Results of `load_defaults`" to the guide.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants