[Laravel-5.0] Add the support for Laravel 5 paginator

[maximebeaudoin]

Related to the issue #150

[GrahamCampbell]

This doesn't work on php 5.3. The dependencies don't install.

[maximebeaudoin]

@GrahamCampbell I know that, it's because illumintate/contracts need php 5.4 if you check at https://github.com/illuminate/contracts/blob/master/composer.json#L12. Unfortunately, we need the Illuminate\Contracts\Pagination\LengthAwarePaginator if we want this to work with laravel 5.

[GrahamCampbell]

Composer has a flag you can run it with to get around this you know. :)

[GrahamCampbell]

Edit the travis file to run composer with --ignore-platform-reqs.

[maximebeaudoin]

Good to know, should i replace travis_retry composer install --no-interaction --prefer-source --dev by this travis_retry composer install --no-interaction --prefer-source --ignore-platform-reqs --dev from the file https://github.com/thephpleague/fractal/blob/master/.travis.yml#L12 ?

[GrahamCampbell]

Yeh, remove --dev from it too. It's deprecated, and is enabled by default.

[maximebeaudoin]

Humm, The command "phpunit --coverage-text --coverage-clover=coverage.clover" exited with 255. when travis try to build one PHP 5.3. Any idea ?

[GrahamCampbell]

Probably because it's scanning files with only php 5.4 valid syntax. :/

[maximebeaudoin]

This weird because i don't see any php 5.4 syntax in illuminate/contracts

Edit: no i see php 5.4 array []

[hipsterjazzbo]

What is left to be done before this can be merged? Is it just figuring out how to make travis do it's thing?

[maximebeaudoin]

@hipsterjazzbo Unfortunately, the package illumintate/contracts require php 5.4 and we can't run the test on travis php 5.3 environment because of this. So, i don't know how to deal with this at the moment.

[hipsterjazzbo]

Hmm. It's not really a matter of just getting travis to pass tests thought right? Like, if a dependency has 5.4 requirement, you won't be able to use it on 5.3 anyway?

In that case, you either have to bump the PHP version requirement, or get rid of the dependency, right?

[GrahamCampbell]

I think we should just drop php 5.3 support totally. It's eol anyway. That would simplify this.

// cc @philsturgeon

[philsturgeon]

Seems fair. I was going to keep dropping 5.3 for 1.0 but lets just knock the old Laravel adapter out and go with this new one.  Sorry to flip flop.

[GrahamCampbell]

@philsturgeon Sounds like a plan. @maximebeaudoin Are you able to update your pull request, or should I send a new one?

[maximebeaudoin]

@GrahamCampbell I'll update mine in few minutes.

[GrahamCampbell]

Great. :)

[GrahamCampbell]

Need to make this ~5.0.

[GrahamCampbell]

Or even remove it if the contracts repo does the job.

[maximebeaudoin]

I'll remove "illuminate/pagination".

[GrahamCampbell]

The suggests block probably need updating too.

[GrahamCampbell]

tbh, i think we should still "suggest" the pagination component itself.

[maximebeaudoin]

Yep, i'll keep it

[GrahamCampbell]

I don't think we should rename the paginator class btw.

[maximebeaudoin]

@GrahamCampbell Yea, i'm wondering too because we have two different type of paginator into Laravel 5, but we can only support LengthAwarePaginator because some method implementation missing in default paginator

[philsturgeon]

Yeah im ok with just supporting length aware. It sounds like the other one would fit in as a cursor better anyway

[maximebeaudoin]

So i keep the name IlluminatePaginatorAdapteror IlluminateLengthAwarePaginatorAdapter ?

[GrahamCampbell]

IlluminatePaginatorAdapter is better imo. I see no reason to change it. The old one was length aware too.

[maximebeaudoin]

I think this is it !

[GrahamCampbell]

You need to bump the minor version in the branch alias too since this is a breaking change.

[GrahamCampbell]

Otherwise, 👍.

[philsturgeon]

IlluminatePaginatorAdapteror please

[philsturgeon]

Sorry my email client is f**ked up.

[philsturgeon]

I'll do that. Thank you!