Giter Club home page Giter Club logo

Comments (17)

lindyhopchris avatar lindyhopchris commented on July 22, 2024 1

Great!

I think for the Eloquent option it would be useful to have the default in the config. The reason being if you are not using Eloquent at all, then it'd be really annoying to always have to provide it to the generator. So maybe there's an --eloquent and a --no-eloquent option (as the naming is clearer) but if neither is provided then the default from config is used. I.e. the option allows you to override whatever the default is in your config.

make:json-api:resource sounds good.

The default namespace (obtained via config) should be JsonApi. This is the namespace I've used for the demo app and all the apps in which I've used this package, so I'd like to keep that as the default. I've never written a Laravel generator before, so don't know if it's possible to pull the application namespace in somehow... i.e. if you've renamed the App namespace.

In terms of App\JsonApi\Schemas\Tasks vs App\JsonApi\Tasks\Schema - I think this should be a pod option in config... with the former being false and the later being true. I'd suggest a default of true because again that's how it is in the demo app.

In terms of the config, I don't think there's too many settings in total (unless you disagree!) so maybe keep them under a generator key in the current json-api.php config. If it starts to get too big we can separate out.

For info, if you're starting any work on this then it should be branched off the develop branch. Keep the questions coming as needed!

from laravel-json-api.

lindyhopchris avatar lindyhopchris commented on July 22, 2024 1

Hi! Have seen the pull request but haven't had time to look through it in detail. Will do so today and get back to you!

from laravel-json-api.

lindyhopchris avatar lindyhopchris commented on July 22, 2024 1

Merged into develop. I need to review to check if this will be released as 0.5.1 or 0.6.0. I believe it can be released as 0.5.1 (which is my preference) as it should be non-breaking but need to double check how it runs if there's no generator config in the json-api.php file.

Things to do:

  • check how the generators behave without config
  • update change log
  • update upgrade guide
  • do release

from laravel-json-api.

lindyhopchris avatar lindyhopchris commented on July 22, 2024 1

Yeah, I went for the generator using a default value when loading the value from config:
fad0559

Not sure about the merging the config because of the amount of other really application specific config that the json-api.php file contains. Will have to try that out in a few different scenarios to see if it causes any problems or if it's ok.

from laravel-json-api.

lindyhopchris avatar lindyhopchris commented on July 22, 2024 1

Yes, had been thinking I'd get to it today but might be tomorrow now

from laravel-json-api.

lindyhopchris avatar lindyhopchris commented on July 22, 2024

Hi! More than happy for you to contribute and adding generators to this package would be a great addition. Thanks for your offer of help!

It'd be great to work out how this will be implemented. I think having separate generators for each "unit" would be really useful, but it'd also be great (if possible) one command that generates all the units at once. I.e. if you're adding Tasks to your application and know you'll need all of the units, you can generate them in one go. (Under the hood that'd just run the separate unit generators.)

I wasn't entirely sure what make:api:resource was in relation to the other generators you listed?

Here were some of my other thoughts:

  • I was wondering whether make:json-api:* would be a better namespace just to ensure no collision with other api packages?
  • For the schema and hydrator commands, it would be useful to have an Eloquent switch so that the generator knows whether to generate one that extends the Eloquent schema/hydrator or not. (Probably Eloquent could be the default with an option to say not Eloquent?)
  • Yes, definitely support the different namespacing conventions that you listed, and yes, that should be configurable via the json-api.php config file

Let me know your thoughts!

from laravel-json-api.

jstoone avatar jstoone commented on July 22, 2024

Seems like our implementation thoughts are very well aligned, that makes me happy. :)

The make:api:resource is actually what you mentioned that will generate the entire suite of units for the given resource e.g Tasks.

  • make:json-api:* sounds like a good idea
  • I would say defaulting to Eloquent is a good idea, and adding a --bare or --raw flag would make the class not extend Eloquent
  • Now it's just figuring out what the default namespace should be. Maybe App\JsonApi\ ?

A few thoughts

  • Also a feature we could check out later is something like the php artisan event:generate, that generates resources based on config namespace, by looking at the eloquent-adapter['map']-attribute
  • We could maybe add generator related configs in a new json-api-generator.php config file?

from laravel-json-api.

jstoone avatar jstoone commented on July 22, 2024

I like where this is going!

  • I agree on changing the option name to the more explicit --eloquent and --non-eloquent
  • Regarding the application namespace, Taylor got us covered! :)
  • I agree that the App\JsonApi\Tasks\* should be default, as well as the base JsonApi.
  • Yes, let's keep the config in json-api.php and extact the generator stuff if needed.
    • I'll be adding the generator stuff at the bottom of the file under the custom adapters, agree?
  • I've branched out from develop and created a branch named artisan-generators, hope that is suitable naming for you as well. :)

Let the hacking begin!

from laravel-json-api.

jstoone avatar jstoone commented on July 22, 2024

Okay, so now the basic generators have been created and are now working. I'll just finish up some refactoring, and then I'll publish the PR.

As of now it only works using eloquent, but I'll post a features list on the PR to track what's missing and what's not.

Questions:

  • There are some of the classes that by default is non-dependant on eloquent and the eloquent related flags will be ignored, is this acceptable behavior?
    • Request
    • Validators
  • Since AbstractSearch seems coupled with Eloquent by itself, should this only be generated when use_eloquent => true ?
  • Are we interested in being able to do the following when creating an entire resource:
    • `php artisan make:json-api:resource Task --only=schema,request
    • `php artisan make:json-api:resource Task --except=search

This is great fun, a lot of good edge-cases to solve :)

from laravel-json-api.

lindyhopchris avatar lindyhopchris commented on July 22, 2024

Great, glad it's fun - this is going to be a fantastic addition, so looking forward to getting it into the package.

In answer to your questions:

  1. If you're not extending from an abstract class it might be best to not have the flags for those two specific commands as they'd be confusing as they'd have no effect. But if you need to just ignore them then that's acceptable and it's fine for the make:json-api:resource to just ignore the flags when it comes to those two specific units.
  2. Yes, only generate an AbstractSearch if Eloquent is true as it is totally coupled at the moment.
  3. Yes, definitely interested in that. It would make sense if you wanted a resource but with only one of the units missing to use the except flag, but I also like the only flag too.

Hope that helps!

from laravel-json-api.

jstoone avatar jstoone commented on July 22, 2024

I agree, generators are such a great tool, and amps up productivity a lot.

As you can see I've created a PR, which is not the finished version. Since it's quite a lot of code to take in at once, I thought publishing while in an early state would make it easier for you (or others) to give feedback while in development.

Looking forward to hearing what you think.

from laravel-json-api.

jstoone avatar jstoone commented on July 22, 2024

Hey @lindyhopchris, the generator is now feature complete and ready to rock! 🎸

I'd love to get your feedback! Even the small things like missing punctuation in docblocks or whatever.

I hope you like it so far!

from laravel-json-api.

lindyhopchris avatar lindyhopchris commented on July 22, 2024

Hey! Thanks for this - I'll hopefully get a chance to look at it today.

from laravel-json-api.

jstoone avatar jstoone commented on July 22, 2024

It will fail. But Laravel gives you an option to merge the users config with the package default config files.
In that way the user will inherit the package configurations, until overridden.

PR is here: #28

from laravel-json-api.

jstoone avatar jstoone commented on July 22, 2024

Sounds super good! I am super excited about seeing my contribution getting merged to develop! Looks like you're about to get ready to release? 🎉

from laravel-json-api.

jstoone avatar jstoone commented on July 22, 2024

Sure thing, don't stress. :)

from laravel-json-api.

lindyhopchris avatar lindyhopchris commented on July 22, 2024

Released as v0.5.2

from laravel-json-api.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.