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

added description interface to extend it #49

Merged
merged 1 commit into from
Oct 16, 2014

Conversation

danieledangeli
Copy link
Contributor

I've added an interface at the Description object and changed as protected its attribute.
I do that because in this way it's allowed to extend the description object. For example if I want to add a different way to fetch the operation, I can pass as an option another method for "defaultCommandFactory" that will receive my custom Description object that implements the DescriptionInterface.

@danieledangeli danieledangeli force-pushed the description-interface branch 2 times, most recently from 9c2c1db to 0fb2c52 Compare September 15, 2014 15:18
@czenker
Copy link

czenker commented Sep 25, 2014

You missed an occurrence in GuzzleHttp\Command\Guzzle\Parameter's constructor. An instanceof check is done there.

Otherwise I really like the idea.

@danieledangeli
Copy link
Contributor Author

Ok I did it. 💃

tl;dr
Just to explain better my first comment:
I made this PR just because I need to extend the description object.
I've seen that it's not a final class and then I just asked to myself: "why private method and not interfaces?"

A simple use case can be the ability to fetch a command (e.g "get.post") by name and a version (1.0, 2.0?). So instead keeping a simple array beteween commandName and Operation I'd like to have a commandName that match a set of operations (one for each version).
Just that! I did that with a workaround, so now should be more easy (Hope) 👍

@czenker
Copy link

czenker commented Sep 26, 2014

Thanks daniele.

This patch would also help me. I'm trying to port Swizzle (it creates a Service Description from a Swagger Definition) to Guzzle4, but am missing the addModel and addOperation methods. It probably does not make sense to have them in guzzle-services, but it should be possible to modify the Definition class.

I'm not sure if we are living in a democracy here, but I would give this patch a 👍 😎

@danieledangeli
Copy link
Contributor Author

Oh that's a good idea! Hope they can accept this path 😞

description interface

fixed op

stashed parameters

Update DescriptionInterface.php

changed attribute to private
mtdowling added a commit that referenced this pull request Oct 16, 2014
added description interface to extend it
@mtdowling mtdowling merged commit 75ae99f into guzzle:master Oct 16, 2014
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.

3 participants