Warning BC break, remove the automatic `__init()` calling from `lithium\core\Libraries`. #985

Merged
merged 1 commit into from Nov 21, 2013

9 participants

@jails
Union of RAD member

No description provided.

@Ciaro

My hero <3

@mikegreiling

I'm curious what this patch does exactly? Was there an issue with this as related to error handling or something?

@jails
Union of RAD member

It's to make lithium works out of the box with the composer autoloader.
You can find more information here and here

It's more a workaround because I don't see any other alternative to the auto __init() call.
But since the reset() mehod stay idempotent this syntax will works the "same way" imo.

@nervetattoo
Union of RAD member

Twelve thumbs up!

@gwoo
Union of RAD member

Can we use multiple autoloaders?

@jails
Union of RAD member

Imo PSR-0 has been designed precisely to avoid that. Afaik In addition to PSR-0 autoloading, classmap is also supported, but this won't solve this issue anyway.

@nateabele
Union of RAD member

This is a pretty big breaking change. I'd like to hold it off at least till after 1.0.

@nervetattoo
Union of RAD member

I'd weigh in that making Lithium fully compatible with other PSR-0 autoloaders would be a big plus for 1.0 — or, rather, I think that not being compliant is a big negative for the first actual release.

@jails
Union of RAD member

All in all, I don't think the change is that big. There's already a lot of BC break since 0.1 so people will need to update their code to take befenefit of the 1.0 anyway.
Imo it'll be easier to fill in the li3's users about the changes than all composer's users about the way they'll need to deal with lithium 1.0.

@djordje

It would be nice to make lithium fully compatible with composer's auto loader, but we should first solve problems with __init() in static classes.
This change make a lot of difference in writing lithium libraries that expect something on initializing static classes...

@jails
Union of RAD member

Hehe @djordje you have to decide which side we are on ;-) !
Being fully compatible with composer's auto loader means no auto __init() call.

Ok, maybe we can keep the auto __init() call of the li3's autoloader but simply let the core not using it. This way non-Composer users can still use the li3's autoloader (i.e. with __init() feature) and in the same time let li3 being fully fonctionnal with the Composer's autoloader ?

@djordje

I believe we should stick to one auto loader and that one should be composer's.
But we should adapt static classes in lithium first.

Maybe to drop __init() and make classes that should be initialized inherit from lithium\core\Object with some static facades, something like laravel?

@gwoo
Union of RAD member

I do not use composer. The static classes already extend StaticObject.

@leek

❤️

@nateabele nateabele merged commit 1319644 into UnionOfRAD:dev Nov 21, 2013

1 check passed

Details default The Travis CI build passed
@agborkowski

any eg. use li3 with composer ?

maybe shall we li3 plugin/libraries command update to composer library index, i think it will be powerful to speed up growth for new libraries ?

@nateabele
Union of RAD member

@agborkowski I'm not sure what you mean. Like an alias for composer update?

@agborkowski

@nateabele maybe more complex solution for create/update/publish (for eg to https://packagist.org/) new libraries

li3 composer create li3_newLib (creates skeleton)
li3 composer update (update like composer update)
li3 composer publish li3_newLib (push to packagist ?)

@nateabele
Union of RAD member

@agborkowski li3 composer update sounds like it would just be an alias for composer update, so probably not worth it IMO. What about li3 composer create? What would that do differently from li3 library extract?

@warrenseymour warrenseymour referenced this pull request in UnionOfRAD/li3_quality Jan 2, 2014
Merged

Remove Rules::__init() #79

@jails jails deleted the jails:refactor/remove-init branch Aug 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment