Fix Railtie / Rails initializers

[haffla]

Hello! First of all thanks for this gem.
I've noticed a problem with the Railtie initializers.

WebValve.enabled? is called inside the Railtie class. The problem with this is that user defined environment variables such as WEBVALVE_ENABLED might not be set when this class is loaded. This is true for users of Figaro or users who dynamically set env vars in an initializer or config file. So the call to enabled? should be inside the initializers.

Moreover Rails.logger (which might be used in enabled? is not initialized at this point. It is actually not initialized when webvalve.set_autoload_paths runs. To solve this I added the safe navigation operator in enabled?.

[nanda-prbot]

@haffla needs to request domain and platform reviewers.

[samandmoore]

Thanks for submitting this PR. I left a few thoughts.

[haffla]

Can I ask why logger= is delegated to Rails? I don't see it used anywhere? Also wouldn't this change the Rails logger when doing WebValve.logger = some_logger? Is this intended behaviour?

[haffla]

Don't merge yet, I found a small problem with this!

[smudge]

Delegating logger= to Rails is probably not intended (since it would lead to pretty surprising behavior) and is probably safe to remove. (@samandmoore?)

(Edit: or, rather than removing, we could define it directly and allow it to set a new logger for just Webvalve)

[samandmoore]

Delegating logger= to Rails is probably not intended (since it would lead to pretty surprising behavior) and is probably safe to remove. (@samandmoore?)

yea, i don't think i thought that one through enough. the fancy logger stuff that's going on here is meant to allow for use of Rails.logger when this library is used with Rails and a default logger or custom logger when used with any other framework.

[haffla]

This part of the code is not covered by the specs and I don't know yet how to test this using a real Rails application. The dummy project in spec/dummy is not in use is it?

[haffla]

Should logger= be made available for both Rails and non Rails projects. Then we could remove the delegation of logger= to Rails.

[samandmoore]

This part of the code is not covered by the specs and I don't know yet how to test this using a real Rails application. The dummy project in spec/dummy is not in use is it?

i believe the spec/dummy app should be used by the test suite. but yea, we don't have any real integration tests right now.

there's an example sinatra app in the examples directory that can be used to verify that we didn't break anything there.

Should logger= be made available for both Rails and non Rails projects. Then we could remove the delegation of logger= to Rails.

yea. let's do that.

[haffla]

@samandmoore is this OK now?

[samandmoore]

looks good. thank you for the submission.

/domain @samandmoore
/platform @samandmoore
<<domainlgtm platformlgtm

[nanda-prbot]

Approved! 🙌 👻 💫

[samandmoore]

@haffla i will look into releasing a new version tomorrow. thanks again.

[samandmoore]

@haffla took a bit longer than expected, but v0.9.7 is released! thanks again.

[haffla]

Thanks a lot for your support!