Devise: [DOCUMENTATION] In the initializer, why are some settings not commented out?

[rlue]

I just started playing with Devise for the first time today. In going through config/initializers/devise.rb, I noticed that most configuration settings are the same as the defaults provided in Devise's lib/devise.rb.^[1] Many of the settings are also commented out, but there appears to be no rhyme or reason as to which ones.

As a new user, I found this very confusing: I had to dig into Devise's source code to confirm that these default values were, in fact, mirrored in the module Devise definition, and that it would be safe to comment them out. At first glance, my assumption was that any uncommented setting _had_ to be manually configured, or else the gem would not work.

Is there a reason for this arrangement that I've somehow missed? Perhaps this was intended as a convenience for experienced users, to remind them of which settings might be more likely to require attention? In any case, that was not my initial interpretation, and getting to the bottom of this question created an unnecessary obstacle in getting started.

In the interest of facilitating adoption for new users, I'd like to suggest that, where reasonable, all default settings in the Devise initializer be commented out from the start. Thoughts?

---

Note 1: Some exceptions to this rule (like @@secret_key or @@mailer_sender) make sense — they should be unique for each application, so they default to nil. Others (like @@paranoid or @@allow_unconfirmed_access_for), are quite confusing — what reason is there to break the pattern established through the rest of the file?

Combined with the inconsistencies described above, these small and unpredictable exceptions make it difficult for a beginner to make reasoned inferences about how the configuration process is designed.

[rlue]

I've also taken a stab at editing the descriptions for each setting, partly to clarify my own understanding and partly to make it easier for new users to get their bearings. Ultimately, I'd like to submit my changes as a PR.

It's been very time-consuming, though, so before I go on and try to edit the whole thing, I thought I might submit a sneak peek of my changes here for your criticism, maybe ask for your blessing:

Now

Devise.setup do |config|
  # secret_key -----------------------------------------------------------------
  # By default, Devise generates random tokens using the `secret_key_base`
  # defined in `config/secrets.yml`. If you wish, you may define a custom
  # secret key, for use exclusively by Devise. (Use `rails secret` to generate
  # a new secret key.)
  #
  # WARNING: Changing this key will invalidate all existing authentication
  # tokens in the database (e.g., confirmation, password reset, & unlock).
  #
  # config.secret_key = nil

  # Mailer
  # ============================================================================

  # mailer & parent_mailer -----------------------------------------------------
  # The mailer class, and the one it inherits from.
  #
  # config.mailer        = 'Devise::Mailer'
  # config.parent_mailer = 'ActionMailer::Base'

  # mailer_sender --------------------------------------------------------------
  # The “from:” e-mail address to be used by Devise::Mailer. When defining a
  # custom mailer class that inherits from Devise::Mailer, this value may be
  # manually overridden (e.g., with `default from:`).
  #
  config.mailer_sender = '[email protected]'

  # ORM
  # ============================================================================

  # ORM Adapter ----------------------------------------------------------------
  # Load the appropriate ORM adapter. Devise includes support for Active Record
  # and MongoID out of the box. Support for other ORMs may be available as
  # third-party gems.
  #
  require 'devise/orm/active_record'

  # Authentication
  # ============================================================================

  # authentication_keys --------------------------------------------------------
  # A list of user attributes to be required for authentication.
  #
  # Along with a password, these attributes make up the set of all request
  # parameters (e.g., form fields) which must match an existing database record
  # for authentication to proceed.
  #
  # If you wish to specify optional attributes — if, say, a `username` is
  # required but not unique for all users, and an optional `organization` could
  # be used to tell the difference — you may supply a hash of the format
  #
  #     { username: true, organization: false }
  #
  # instead.
  #
  # config.authentication_keys = [:email]

  # request_keys ---------------------------------------------------------------
  # More user attributes required for authentication.
  #
  # Whereas the attributes above are taken from the `params` hash, attributes
  # specified here are taken from the corresponding methods of Rails' built-in
  # ActionDispatch::Request object.
  #
  # (ActionDispatch::Request provides information about a given HTTP request,
  # such as the originating IP address, host, port number, cookies, MIME type,
  # etc. For more information, refer to the Rails API docs at
  # http://api.rubyonrails.org/classes/ActionDispatch/Request.html)
  #
  # For instance, when a user visits http://blog.plataformatec.com.br/,
  # ActionDispatch::Request#subdomain will return the string 'blog'. Thus,
  # users may be constrained to log in from a given subdomain (or, put another
  # way, they may be identified according to the subdomain they are expected
  # log in from) by:
  #
  #   1. defining a `subdomain` attribute on the User model (with a
  #      corresponding column in the `users` database table),
  #   2. setting this attribute to the desired value, and
  #   3. specifying `[:subdomain]` below.
  #
  # As above, attributes may be made optional by passing a hash instead.
  #
  # config.request_keys = []

  # case_insensitive_keys & strip_whitespace_keys ------------------------------
  # Lists of user attributes whose string values will be normalized — either by
  # downcasing or removing leading/trailing whitespace — before being stored in
  # or retrieved from the database (e.g., to ensure that “[email protected]” and
  # “[email protected] ” match the same database records).
  #
  # config.case_insensitive_keys = [:email]
  # config.strip_whitespace_keys = [:email]

  # params_authenticatable & http_authenticatable ------------------------------
  # Devise uses so-called “authentication strategies” to validate login
  # attempts and other user activity. These strategies are simply classes
  # (e.g., Devise::Strategies::DatabaseAuthenticatable) that implement a common
  # API to report whether a given set of user credentials is valid.
  # (For more information, see https://insights.kyan.com/a1a6b4e2b891)
  #
  # These settings specify whether the authentication strategies used in your
  # application may read user credentials supplied from HTTP request parameters
  # (i.e., REST actions — the standard for most web applications) or HTTP Basic
  # authentication headers, respectively.
  #
  # If you wish to limit which authentication strategies may access which
  # sources of user credentials, you may supply a list of allowable strategies
  # for each setting instead, such as [:database] for DatabaseAuthenticatable.
  #
  # config.params_authenticatable = true
  # config.http_authenticatable   = false
...

Before

Devise.setup do |config|
  # The secret key used by Devise. Devise uses this key to generate
  # random tokens. Changing this key will render invalid all existing
  # confirmation, reset password and unlock tokens in the database.
  # Devise will use the `secret_key_base` as its `secret_key`
  # by default. You can change it below and use your own secret key.
  # config.secret_key = 'ee40f65921e78f4c3eb4db6447f2c7f3167a27a41fe69b878e811012b2aab72121862e67120ef51d64c00c8d34663716aa5ea7b906a40c9e25cbe5cccbce6f9b'

  # ==> Mailer Configuration
  # Configure the e-mail address which will be shown in Devise::Mailer,
  # note that it will be overwritten if you use your own mailer class
  # with default "from" parameter.
  config.mailer_sender = '[email protected]'

  # Configure the class responsible to send e-mails.
  # config.mailer = 'Devise::Mailer'

  # Configure the parent class responsible to send e-mails.
  # config.parent_mailer = 'ActionMailer::Base'

  # ==> ORM configuration
  # Load and configure the ORM. Supports :active_record (default) and
  # :mongoid (bson_ext recommended) by default. Other ORMs may be
  # available as additional gems.
  require 'devise/orm/active_record'

  # ==> Configuration for any authentication mechanism
  # Configure which keys are used when authenticating a user. The default is
  # just :email. You can configure it to use [:username, :subdomain], so for
  # authenticating a user, both parameters are required. Remember that those
  # parameters are used only when authenticating and not when retrieving from
  # session. If you need permissions, you should implement that in a before filter.
  # You can also supply a hash where the value is a boolean determining whether
  # or not authentication should be aborted when the value is not present.
  # config.authentication_keys = [:email]

  # Configure parameters from the request object used for authentication. Each entry
  # given should be a request method and it will automatically be passed to the
  # find_for_authentication method and considered in your model lookup. For instance,
  # if you set :request_keys to [:subdomain], :subdomain will be used on authentication.
  # The same considerations mentioned for authentication_keys also apply to request_keys.
  # config.request_keys = []

  # Configure which authentication keys should be case-insensitive.
  # These keys will be downcased upon creating or modifying a user and when used
  # to authenticate or find a user. Default is :email.
  config.case_insensitive_keys = [:email]

  # Configure which authentication keys should have whitespace stripped.
  # These keys will have whitespace before and after removed upon creating or
  # modifying a user and when used to authenticate or find a user. Default is :email.
  config.strip_whitespace_keys = [:email]

  # Tell if authentication through request.params is enabled. True by default.
  # It can be set to an array that will enable params authentication only for the
  # given strategies, for example, `config.params_authenticatable = [:database]` will
  # enable it only for database (email + password) authentication.
  # config.params_authenticatable = true

  # Tell if authentication through HTTP Auth is enabled. False by default.
  # It can be set to an array that will enable http authentication only for the
  # given strategies, for example, `config.http_authenticatable = [:database]` will
  # enable it only for database authentication. The supported strategies are:
  # :database      = Support basic authentication with authentication key + password
  # config.http_authenticatable = false
...

[rafaelfranca]

The reason for having uncommented configurations in the generated is backward compatibility. If the internal defaults on device change and you have a commend value in your application, when you upgrade the gem to the new version your application automatically will use the new value without you knowing and it may break your application because it rely on the previous value.

If we generate the file with uncommented values, even that it reflect the actual default, when you upgrade devise to a version where this default changed your application will still work and you only change the configuration to the new value if you want and when you want.

[rlue]

That makes sense.

But why are most of the configuration values commented out then? Shouldn't they _all_ be uncommented to begin with? (Again, I'm just trying to think in terms of what would be easiest for a first-time user to intuit.)

[rlue]

If it would be better to have all config values uncommented by default, I'd be happy to submit a PR.

[rafaelfranca]

I'm up to it, but better wait @lucasmazza since he may have more context why they are commented out.

[tegon]

I'm also not sure why they're commented out, but in some projects I've worked on, the developers would delete all the commented parts to keep the initializer with only the configurations needed.
I guess if we had all the settings uncommented it could look like devise needs all those values set in the initializer to work properly - which isn't true because they're configurated by default. What do you think?

[rlue]

I agree with you 100%, @tegon — commenting out config values is a good way to subtly communicate 1) that there are defaults in place, and 2) what those defaults are.

But speaking to @rafaelfranca's point, if the defaults change in a new version, then those commented-out values become lies, which could make for a debugging nightmare.

My understanding of semver is that anything that breaks backwards compatibility requires a major version bump. I know it's a grey area, but doesn't that include changing the initializer defaults? As such, my preference would be for:

1. commented-out defaults everywhere,
2. a major version bump when defaults change, and
3. _maybe_ a cautionary note about it at the top of the initializer file.

[rafaelfranca]

I still prefer uncommented defaults so we can actually change defaults in minor releases.

[rlue]

If that's the case, it'd be helpful to have a note at the top of the initializer about it. Some poor sap might go through the whole file line by line (like I did) and comment out all the un-commented default settings just for the sake of internal consistency.

I think the decision to un-comment certain settings is a good one, but if users are not informed about it, they're liable to reverse it without understanding the consequences.

[tegon]

Sounds good.

@rlue Do you want to open a pull request adding that documentation? If not, let me know so I can assign to someone else.

Thanks.

[rlue]

I can get started on this next week. Happy holidays!

[tegon]

Hey @rlue, just pinging to see if you still intend to work on this.

If you can't, just let me know so we can see if someone wants to do it.

[rlue]

Jesus, sorry for that epic delay! PR #5142 has been submitted to address this issue. Thanks!

[tegon]

No problem, thank you!