Improved generated config/application.cr commented code to work once uncommented.

[elorest]

Description of the Change

1. Uncommented PORT env logic by default and converted to int32.
2. Added valid secret_key_base to commented out secret_key_base settings code.
3. Added checks so that ENV vars are only applied if they exist. That way the standard practice of using default settings in development and test but ENV vars in production will still work without constant code changes.
4. Changed example settings values to match correct type for setting (bool, int32) etc. ENV vars are all strings and fail by default.

Benefits

Within Rails and Phoenix every ENV override has default values that are used instead if they're not set.

For example: If a PORT isn't set it goes to 3000, if an RAILS_ENV isn't set it goes to development, if DATABASE_URL isn't set it uses settings from database.yml.
I don't think that it's clear to users that uncommenting the code in application.cr will work differently.
For instance if settings port is 3000, ENV["PORT"] should only override that if ENV["PORT"] has a value. Otherwise we just end up with a project that doesn't work.

settings.port = ENV["PORT"] will fail in too many situations that aren't expected by people coming from rails/laravel, pheonix, hanami.

Instead it should be settings.port = ENV["PORT"].to_i if ENV["PORT"]?.

Otherwise we just end up with a weird situation were we load values in and immediately overwrite them with values that may not even exist.

We now end up with example code that will work out of the box, which will result in fewer complaints.

[marksiemers]

Just a question about whether or not we should provide hard-coded defaults for any missing ENV variables.

[marksiemers]

Should there be a hardcoded fallback here?

settings.process_count = ENV["PROCESS_COUNT"]? ? ENV["PROCESS_COUNT"].to_i : 1 

[faustinoaq]

We already have process_count = 1 here

Notice ]? , so returns nil if not found and setting attribute is not assigned.

[elorest]

Yes. There is already a fall back for all of them. They just weren't being used if the environment var was missing.

[marksiemers]

Same question here, should there be a default, or do we want it to fail loudly if that ENV variable isn't there?

[faustinoaq]

We're already using port = 3000 here

:ditto:

Do we want it to fail loudly if that ENV variable isn't there?

I will not fail because is checking PORT availability first if ENV["PORT"]?

[elorest]

It's pointless to have a default if we're going to overwrite it with a non existent environment var.

I strongly feel that we should follow the same pattern that all the other Frameworks we have used for the last 10 years follow.

Use the default unless the environment variable exists.