search

SaturnFramework / Saturn

Opinionated, web development framework for F# which implements the server-side, functional MVC pattern
https://saturnframework.org
MIT License
714 stars 108 forks source link

*_from_config custom operations in application CE #150

Closed mvsmal closed 2 years ago

mvsmal commented 5 years ago

WebHost.CreateDefaultBuilder() loads application configuration from several places by default:

It aggregates them in IConfiguration instance available in the ServiceCollection.

The idea of this PR is to provide an already loaded configuration to the configuration functions for simplification of environment differences handling.

With this PR I would also like to open a discussion about other custom operations which might deserve similar approach, such as use_config or forse_ssl, but the implementation there would be different.

mvsmal commented 5 years ago

@Krzysztof-Cieslak Should I continue on this? What are your thoughts?

Krzysztof-Cieslak commented 5 years ago

Hey, thanks a lot, and sorry it took me a while - I've been bit busy with the work stuff and traveling for last month.

In principle - I like it, it really seems like a useful addition. But I'm getting bit worried about number of custom operations (especially given that tooling around them is not perfect so discovery is bit tricky) and naming (i.e. use_jwt_auth_from_configuration and use_jwt_authentication_with_config are too similar for my liking). I need to think about that... but probably renaming existing _with_config to _with_builder would be enough.. hmm.

mvsmal commented 5 years ago

I am fine with rename the operations to smth shorter. I am not insisting on the naming here, more on the functionality since I really miss it in my projects. Your suggestion would be a breaking change, but maybe it's fine for version 1.0

Krzysztof-Cieslak commented 5 years ago

Your suggestion would be a breaking change

I'm definitely open to do breaking changes before 1.0 release.

tforkmann commented 2 years ago

@mvsmal Is this still required?

mvsmal commented 2 years ago

@tforkmann seems like we can live without this 😃