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

fix: fix Lumen v5.5 compatibility #17

Merged
merged 1 commit into from
Oct 24, 2017

Conversation

joelhy
Copy link
Collaborator

@joelhy joelhy commented Sep 7, 2017

  • fix fire method Lumen v5.5 compatibility

According to https://laravel.com/docs/5.5/upgrade, the fire method has been changed to handle in Lumen v5.5,
which break lumen-generator in Lumen v5.5.

In Laravel v5.4 Illuminate/Console/Command.php Line 180, there is handle method support:

$method = method_exists($this, 'handle') ? 'handle' : 'fire';

So it just need to change fire method name to handle.

  • fix artisan route:list Lumen v5.5 compatibility

In Lumen v5.5 router methods is not directly attached to Application. To get Router object, we need to use $app->router.

This fix add Lumen v5.5 compatibility support, and do not break Lumen v5.4 support.

@jgrossi
Copy link

jgrossi commented Sep 11, 2017

@krisanalfa can you merge this PR? it's really useful :-)

@Barry-Fisher
Copy link

Barry-Fisher commented Sep 20, 2017

I second this - Please merge this PR

For now as a workaround I have used the diff from this PR via the composer-patches package:

composer require cweagans/composer-patches

and added this to composer.json:

    "extra": {
        "patches": {
            "flipbox/lumen-generator": {
                "fix Lumen v5.5 compatiblity: https://github.com/flipboxstudio/lumen-generator/pull/17": "https://github.com/flipboxstudio/lumen-generator/commit/c80803f8a57494a3f0f81de072e2878b00f7711c.patch"
            }
        }
    },

then removed the lumen-generator package:

rm -Rf vendor/flipbox/lumen-generator/

and finally added it back with:

composer update --prefer-source flipbox/lumen-generator

The patch is successfully applied if you see something like:

Gathering patches for root package.
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 1 install, 0 updates, 0 removals
Gathering patches for root package.
Gathering patches for dependencies. This might take a minute.
  - Installing flipbox/lumen-generator (5.4.3): Cloning 93e6647bf2 from cache
  - Applying patches for flipbox/lumen-generator
    https://github.com/flipboxstudio/lumen-generator/commit/c80803f8a57494a3f0f81de072e2878b00f7711c.patch (fix Lumen v5.5 compatiblity: https://github.com/flipboxstudio/lumen-generator/pull/17)

Generating optimized autoload files

Note: I was still getting an issue with php artisan route:list but I'll raise a separate issue for that and link over once done. UPDATE: Here's the separate issue: #19

@jgrossi
Copy link

jgrossi commented Sep 20, 2017

@barryrld for now, I'm using @joelhy's branch :-)

"repositories": [
    {
        "type": "vcs",
        "url": "https://github.com/joelhy/lumen-generator"
    }
],
"require-dev": {
    "flipbox/lumen-generator": "dev-fix/lumen5.5_compatiblity",
},

@Barry-Fisher
Copy link

Thanks @jgrossi! I've updated my composer.json to use @joelhy's dev branch as you suggested.

I've created a new issue for the php artisan route:list issue here: #19

* fix `fire` method Lumen v5.5 compatibility

According to https://laravel.com/docs/5.5/upgrade, the `fire` method has been changed to `handle` in Lumen v5.5,
which break `lumen-generator` in Lumen v5.5.

In Laravel v5.4 Illuminate/Console/Command.php Line 180, there is `handle` method support:

```php
$method = method_exists($this, 'handle') ? 'handle' : 'fire';
```

So it just need to change `fire` method name to `handle`.

* fix `artisan route:list` Lumen v5.5 compatibility

In Lumen v5.5 router methods is not directly attached to Application. To get `Router` object, we need to use `$app->router`.

This fix add Lumen v5.5 compatibility support, and do not break Lumen v5.4 support.
@joelhy joelhy changed the title fix: fix Lumen v5.5 compatiblity fix: fix Lumen v5.5 compatibility Sep 21, 2017
@joelhy
Copy link
Collaborator Author

joelhy commented Sep 21, 2017

@barryrld @jgrossi
I have fixed artisan route:list Lumen v5.5 compatiblity.

@aqidd aqidd merged commit d5d3aca into flipboxstudio:develop Oct 24, 2017
@aqidd
Copy link
Member

aqidd commented Oct 24, 2017

Hi @joelhy thanks a lot for your contributions on this project.
Currently we don't have that much manpower to maintain this repository. Would you like to be a collaborator?

Cheers

@joelhy
Copy link
Collaborator Author

joelhy commented Oct 24, 2017

@aqidd
Yes, it's my pleasure to contribute to this project.

@aqidd
Copy link
Member

aqidd commented Oct 24, 2017

Thanks!!

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

Successfully merging this pull request may close these issues.

None yet

4 participants