Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Fuel::always_load() method contains old code that needs to be fixed #907

Closed
axelitus opened this issue Apr 4, 2012 · 2 comments
Closed
Labels

Comments

@axelitus
Copy link
Contributor

axelitus commented Apr 4, 2012

The Packages are loaded now in the Fuel::init() method L199. There's even a comment in the Fuel::always_load() method stating this, but instead of getting rid of the code that loaded the packages in earlier versions, there's just a patch to prevent them from loading. Why not just delete that code? It is not needed anymore. It's slightly misleading having two different codes that do the same in two different places and blocking the execution of one of them. Is there a reason I'm not seeing that this was done like this?

What should be done is leave the code for package loading in the init() method and delete the old code in the always_load() method or replace the old code entirely with the new code inside the always load() method instead of the init() method. I think the best would be to move the new code inside the always_load() method.

@jschreuder
Copy link
Contributor

The idea always was to also make always_load() fit to be used with something other than the default application array. Of course the default usage without input uses the application's always_load config value and loads those. But for the application the packages were already loaded in the original init, and thus when the default input is loaded from there the packages are removed - which wouldn't happen if you called always_load() with an array as param.
The reason the packages are loaded earlier is because it wouldn't be possible to extend classes from packages otherwise, which this enables for any class used between the packages loading and the call to always_load().

Now I don't remember us implementing it anywhere else, but there's no reason to disallow this. So it should be fixed by rewriting it to use Package::load().

@WanWizard
Copy link
Member

Not entirely the case, altough I have to admit the true reason is disguised pretty well.

Point is that you call call Fuel::always_load() with an array, to have your custom "always_load" array that's not in the config. And if you pass an array, packages in that array should be loaded.

It should call Package::load() there though, and not add_package() which is deprecated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants