LemmyNet
Dont pass settings as parameter
We are passing settings around as function parameters in many places, or accessing via context.settings(). That is completely unnecessary because we can always access the static SETTINGS.
i think maybe it makes more sense to go the other way and make the SETTINGS thing private and always access through context
one reason would be that you can't cleanly mock global state
Making SETTINGS private isnt possible because its defined in lemmy_utils, but context is in lemmy_api_common. So to allow access from the other crate it must be public, and then it can be used directly from anywhere in the code. Besides we dont have any tests which mock this.
I agree with @phiresky, we should avoid globals as much as possible, and always pass what's necessary via function params.
This is a long video about functional programming, but it really internalized for me how much better things can be when you code to avoid problems of globals and state.
I'm thinking we should go the other way with this: make settings (and all the other public global statics), a part of the LemmyContext initialization, to make sure they're only accessed through it.
If no one's opposed I'll make an issue and work on that.
@dessalines Like I said in my previous comment this is not possible because they are defined in different crates. For LemmyContext to access Settings it needs to be public, which means the entire code can access it. So it makes sense to remove it from context if that can be bypassed anyway.
It'd probably be better to make Settings::init public, then move the LazyLock (if we still want to keep it, its probably not necessary tho), to the LemmyContext init. Then make sure in code that Settings::init is only called once.
IMO its dangerous to be passing around this global static which requires reading files and env vars, and especially calling it from inside functions without it being explicitly passed.
Then we need to move LemmyContext into lemmy_utils https://github.com/LemmyNet/lemmy/issues/5200