Skip to content
This repository has been archived by the owner on Sep 7, 2021. It is now read-only.

Added a command for create transformer #21

Closed
wants to merge 3 commits into from
Closed

Added a command for create transformer #21

wants to merge 3 commits into from

Conversation

fd6130
Copy link

@fd6130 fd6130 commented Oct 26, 2020

This PR is for solving the issue from #20.

@samjarrett
Copy link
Owner

Does this break anything if the maker bundle is not installed?

@fd6130
Copy link
Author

fd6130 commented Oct 26, 2020

Does this break anything if the maker bundle is not installed?

It use some of the classes such as Str, Generator and AbstractMaker from maker-bundle, so the command won't work if user don't have maker-bundle in their project i think.

@fd6130 fd6130 marked this pull request as draft December 6, 2020 10:44
@fd6130
Copy link
Author

fd6130 commented Dec 6, 2020

Any feedback on this one?

And another question: Should i change it to "ask for entity class name, and generate a new transformer class in App\Transformer\YourEntityTransformer.php for example`, or "ask for transformer class name and generate a new transformer class" instead ?

@GPHemsley
Copy link

I think most other makers just ask for the name and don't bother trying to perform any additional logic, so that'd probably the best route here. Besides, I'm not sure there's much difference in terms of the output? I suppose you could get fancy with it, but that might not meet everyone's needs.

@fd6130
Copy link
Author

fd6130 commented Dec 10, 2020

I think most other makers just ask for the name and don't bother trying to perform any additional logic, so that'd probably the best route here. Besides, I'm not sure there's much difference in terms of the output? I suppose you could get fancy with it, but that might not meet everyone's needs.

Hmm so i will stick to the "ask for transformer class name and generate a new transformer class", user will fill in whatever they need in that new class. Will update this later.

@GPHemsley
Copy link

Circling back to this...

@fd6130: It looks like there's an opportunity here to prompt for includes like make:entity prompts for properties: then you can populate availableIncludes and defaultIncludes, plus create the corresponding includeX method.

Thoughts on also doing a make:fractal:serializer? (Seems to be distinct from the Symfony serializer.)

@fd6130
Copy link
Author

fd6130 commented Feb 18, 2021

Circling back to this...

@fd6130: It looks like there's an opportunity here to prompt for includes like make:entity prompts for properties: then you can populate availableIncludes and defaultIncludes, plus create the corresponding includeX method.

Thoughts on also doing a make:fractal:serializer? (Seems to be distinct from the Symfony serializer.)

Ahh almost forgot this PR, I will see what i can do with the "availableInclude" and "defaultInclude" when i have the time.

I think i will remove the prompt of asking user to enter an entity when creating transformer (because not every transformer are bound to an entity).

And also, what is the benefit of "make:fractal:serializer" ? Just create an empty fractal serializer like creating a transformer?

@GPHemsley
Copy link

I think i will remove the prompt of asking user to enter an entity when creating transformer (because not every transformer are bound to an entity).

You could always allow a blank to bypass the prompt, thereby giving the user the option.

And also, what is the benefit of "make:fractal:serializer" ? Just create an empty fractal serializer like creating a transformer?

Yeah, exactly. Since League\Fractal\Serializer\SerializerAbstract is abstract, it has some required methods that need to be implemented for a Serializer to work. Not sure if there's much in the way of options that can be customized at the command prompt level, though. In any case, upon further thought, definitely outside the scope of this PR.

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants