I think I’m missing some context here but why is this required or how do I trigger the warning? It seems weird that we have to define constant like this because loading and reloading should be handled by Rails.
We do this at countless places in the app. Also just start your rails server before this patch and you should see the warning.
Looks like this is because we’re loading the same file twice. I’ve fixed it in https://github.com/discourse/discourse/commit/20bd6d97977a1f9e21c1d671f8b7385600c9ea39.
We do this at countless places in the app.
We shouldn’t have to do
||= on constants so I’ll try and see what is causing it once I have time
@tgxworld I am recommending most people declare constants with
||= because this issue is so common in rails due to the auto loader in development mode. There is no obvious downside that I am aware of.
@eviltrout The downside here is that we have to define Ruby constant in a way that is not how Ruby constants are normally defined. Plus we keep having PRs and commits like this to remove the warnings. I’m also not quite sure if this is a problem with the Rails autoloader because non of the Rails projects I’ve used have to define constants this way. I have a feeling this is something specific to our core code so I’ll spend some time figuring out why.