dingo / api

A RESTful API package for the Laravel and Lumen frameworks.
BSD 3-Clause "New" or "Revised" License
9.32k stars 1.25k forks source link

Laravel 5 AND Lumen Support #213

Closed jasonlewis closed 9 years ago

jasonlewis commented 10 years ago

Greetings API developer.

Okay so I know quite a few people are jumping over to L5 to use all the latest and greatest stuff that it has to offer. That's fine. You can do that. But this package isn't L5 ready any more.

This package will support the next version of Laravel. BUT, right now, I'm waiting for the dust to settle. There's a lot happening in there and I'll change one thing to fix something and before you know it another thing will change. So, for now, I'm going to iron out the remaining issues for 4.x and merge the develop branch over to master and start tagging 0.7.* releases.

I'm hoping to get the package in a somewhat stable condition. My thoughts are to then create a branch specifically for L5 as there's a number of things that will need to be changed in order for it to be compatible.

Any feedback, post it here.

Cheers.


Update 2014-05-17

rowino commented 10 years ago

That's okay. I can help iron out some of the issues on laravel 5 as I'm actively building an api with it.

xLink commented 10 years ago

out of curiosity will be you be back porting any new features / bug fixes you make back to l4's branch?

jasonlewis commented 10 years ago

They will remain identical in features. There's just too much difference between the Laravel core now. On 9 Oct 2014 20:04, "Dan Aldridge" notifications@github.com wrote:

out of curiosity will be you be back porting any new features / bug fixes you make back to l4's branch?

— Reply to this email directly or view it on GitHub https://github.com/dingo/api/issues/213#issuecomment-58481683.

rtorcato commented 10 years ago

L5 isn't even in beta. Just wait a couple more months.

jasonlewis commented 10 years ago

Yes, I'm aware of where Laravel 5 is at in terms of development. :-)

rtorcato commented 10 years ago

lol i mean all the people that are expecting composer packages to be working with an alpha should just chill. Even the new L5 tutorials on Laracasts are already out of date with L5.

jasonlewis commented 10 years ago

Oh yeah. I totally get what you mean mate. It's pretty hectic! Heh.

On Sat, Oct 11, 2014 at 1:34 AM, Richard Torcato notifications@github.com wrote:

lol i mean all the people that are expecting composer packages to be working with an alpha should just chill. Even the new L5 tutorials on Laracasts are already out of date with L5.

— Reply to this email directly or view it on GitHub https://github.com/dingo/api/issues/213#issuecomment-58663978.

niallobrien commented 10 years ago

Lol, I'd just installed it in a fresh L5 installation and was wondering why I was seeing errors. :)

jasonlewis commented 10 years ago

Yep! Not going to work for a little while yet. On 16 Oct 2014 11:08, "niallobrien" notifications@github.com wrote:

Lol, I'd just installed it in a fresh L5 installation and was wondering why I was seeing errors. :)

— Reply to this email directly or view it on GitHub https://github.com/dingo/api/issues/213#issuecomment-59296382.

niallobrien commented 10 years ago

Ah damn...Ah well, I'll continue working on the client-side and mock the backend for now. Are you thinking 2015 before it's L5 ready?

jasonlewis commented 10 years ago

Hm depends when L5 is ready I suppose. We'll see! On 16 Oct 2014 20:28, "niallobrien" notifications@github.com wrote:

Ah damn...Ah well, I'll continue working on the client-side and mock the backend for now. Are you thinking 2015 before it's L5 ready?

— Reply to this email directly or view it on GitHub https://github.com/dingo/api/issues/213#issuecomment-59336118.

rowino commented 10 years ago

@niallobrien, I had the same problem (developed an api with unstable L5). What I ended up doing was limiting it to a specific commit that didn't break much. I went further to remove a line in the repo that caused an error with method injection and so got my app working with L5 until it becomes stable. Hopefully not much will change before then though already there are numerous changes. If you can wait for the November release very well. Otherwise, you can limit it to a commit that works for you develop your app then in November upgrade it.

Commit: 4c0600ce19d706ccce74df9b25c469948b23c92d Commented out line: Illuminate\Foundation\Providers\FormRequestServiceProvider line 38

niallobrien commented 10 years ago

@ralphowino Thanks. :)

joshhornby commented 10 years ago

I 100% understand why you arent supporting L5 as of yet, but any chance of a develop branch with support for illuminate/support 5? I know it has been removed in a commit a few days ago https://github.com/dingo/api/commit/04adf54567f39e06df89cd3eb5aeebaec9998b0e but just would really like to hack away at something :)

MartijnThomas commented 9 years ago

@jasonlewis I'm not sure if you were holding back on getting this package ready due to the removed package registration (If not, you can discard this message), but this seems to be fixed (according to Taylor at Twitter): https://twitter.com/taylorotwell/status/553007842337103872.

Not exactly fixed, since the 'package' method will not return though.

jasonlewis commented 9 years ago

I'm yet to start working on any form of L5 compatibility. Will keep this issue updated when I start.

GrahamCampbell commented 9 years ago

@jasonlewis I'd recommend using orchestra's "config" and "support-providers" components. I'm doing to be using them in all my laravel packages. I want to push them to the standard for laravel 5 packages.

// cc @crynobone

jasonlewis commented 9 years ago

Will take a look, cheers @GrahamCampbell.

joshhornby commented 9 years ago

Any ETA on when L5 development will start? Very eager to use this in a laravel 5 app :)

Taylor has has pretty much said this is stable now and the release is set for next week.

jasonlewis commented 9 years ago

Soon. I'll wait for the release next week before I push anything public. There's a few things I want to tidy up first before I branch it off to make it compatible.

kennonb commented 9 years ago

Can't wait! I love this package. :) :+1:

hipsterjazzbo commented 9 years ago

@jasonlewis How big of a job are you anticipating? Like, just enough to get it going in L5, or are you expecting a bigger refactor will be necessary?

joshhornby commented 9 years ago

Good question @HipsterJazzbo, I am about to start a pretty big project and would love to use this package as the base for the API. Taylor tweeted that L5 will be out either tomorrow or day after.

jasonlewis commented 9 years ago

I'm not entirely sure to be honest. I've got a bit on my plate at the moment so haven't had a chance yet to make anyway headway. I'll be sure to update here though once I do.

hipsterjazzbo commented 9 years ago

Cool. I'd love it if you just chuck a 4.2.*|5.0.* in there, so that us guys could start hacking on it. I've been doing updates and opening PRs for days.

jasonlewis commented 9 years ago

I'll throw up a develop branch in a minute so you guys can have a look. I warn you though. It'll be a broken mess that I doubt will even work.

hipsterjazzbo commented 9 years ago

It certainly won't work. I'm keen to help out though, as much as I can without getting into things that are creator decisions.

jasonlewis commented 9 years ago

All good. Give me a couple and I'll have it up. :smile:

hipsterjazzbo commented 9 years ago

Hovering over the enter key:

$ composer require dingo/api

jasonlewis commented 9 years ago

Should be able to require dev-develop now. I hope.

hipsterjazzbo commented 9 years ago

BOOM. Thanks a bunch.

jasonlewis commented 9 years ago

You're welcome. :smile:

ghost commented 9 years ago

L5 has been released officially. I hope Dingo API will be released with L5 compatibility soon.

joshhornby commented 9 years ago

Are you open to the community helping out to port this across @jasonlewis?

primathon commented 9 years ago

Can't wait until this gets up and running :) been looking forward to this packing being upgraded for quite a while.

jasonlewis commented 9 years ago

@joshhornby I'm more then happy to have people send PRs to the develop branch for some L5 compatibility.

I don't have the time right this very minute. Hoping to look into it more next week. Sorry guys, just been a little busy with other things.

Feel free to send some PRs though, develop branch should be your target.

foxted commented 9 years ago

I tried to execute the tests on each "component" using these dependencies:

    "require": {
        "php": ">=5.4.0",
        "illuminate/support": "~5.0",
        "league/fractal": "0.11.*"
    },
    "require-dev": {
        "lucadegasperi/oauth2-server-laravel": "4.0.*@dev",
        "tymon/jwt-auth": "dev-laravel-5",
        "illuminate/routing": "~5.0",
        "illuminate/events": "~5.0",
        "illuminate/auth": "~5.0",
        "illuminate/database": "~5.0",
        "illuminate/pagination": "~5.0",
        "illuminate/console": "~5.0",
        "illuminate/filesystem": "~5.0",
        "phpunit/phpunit": "~4.0",
        "mockery/mockery": "~0.9",
        "squizlabs/php_codesniffer": "~2.0"
    }

Only the Http & Routing folders tests don't pass, so I assume that at least the Http & Routing component of the package need to be changed to make the whole package compatible L5.

I could be entirely wrong, but I will try to work on any issue I can solve and submit a PR.

GrahamCampbell commented 9 years ago

fractal doesn't support L5 just yet atm

joshhornby commented 9 years ago

Is there a ETA for L5 on fractal? I had a look the other day and doesn't seem much going on over there @GrahamCampbell

GrahamCampbell commented 9 years ago

Fractal shouldn't take too long. The changes are quite small. We basically just need a new pagination adapter.

kodeine commented 9 years ago

Since Jason is busy, no one proposed PR for L5 compatibility yet?

stygiansabyss commented 9 years ago

@GrahamCampbell looks like the pagination adaptor is done now yes? If so, I may try seeing if I can get this working on L5 and submit a PR for it.

GrahamCampbell commented 9 years ago

@stygiansabyss You're going to need to change "league/fractal": "0.11.*" to "league/fractal": "0.12.*" when you do. :)

kennonb commented 9 years ago

So I've been toying around with this for a bit. Still not really getting anywhere. I thought I had, since I got the routes to work in Artisan, but they aren't properly resolving in the browser.

https://github.com/kennonb/api/commits/laravel5

foxted commented 9 years ago

Same issue on my side.... https://github.com/foxted/api/tree/develop @kennonb

hipsterjazzbo commented 9 years ago

@kennonb @foxted What specifically is the issue you're having?

kennonb commented 9 years ago

Running artisan api:routes & artisan route:list both return the registered routes but none of the routes actually resolve when trying to hit them from a browser.

Some of the DispatcherTests fail as well in reference to named / action routes. But I can't quite figure out why they're failing. Tracing the named routes shows that they are resolving but the functions to retrieve a named route are returning "null".

I may try and record a quick look at what I'm seeing since this probably isn't making sense. :)

foxted commented 9 years ago

Same problem: I see routes using the artisan command but they do not work in the browser. I thought it could be related to the RouteServiceProvider now included in L5 that gets in the way, but no idea to solve this issue so far. I was thinking to move auth & rate limiting to middleware as well. Feel free to contribute on my fork if you have any idea, lets make the job easier @jasonlewis ! :palm_tree:

hipsterjazzbo commented 9 years ago

I wonder if we should be doing this a bit at a time? Not trying to all individually rewrite the whole package?

Maybe we should submit little changes against develop, and coordinate on here who's doing what. That way someone can convert filter's to middleware, and someone else can do some other stuff, etc.

Thoughts?

kennonb commented 9 years ago

:+1: I think that's definitely a good idea.