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

Update plugin documentation #104

Merged
merged 1 commit into from
May 4, 2016
Merged

Update plugin documentation #104

merged 1 commit into from
May 4, 2016

Conversation

sagikazarmark
Copy link
Member

No description provided.

@dbu dbu changed the title Update plugin documentation WCM: Update plugin documentation May 3, 2016
@dbu
Copy link
Contributor

dbu commented May 3, 2016

i added WCM to the title (waiting for code merge).
we need to merge php-http/client-common#14 first.

content wise i agree, looks good. can you check travis-ci?

@@ -4,7 +4,7 @@ Building Custom Plugins
When writing your own Plugin, you need to be aware that the Plugin Client is async first.
This means that every plugin must be written with Promises. More about this later.

Each plugin must implement the ``Http\Client\Plugin\Plugin`` interface.
Each plugin must implement the ``Http\Client\Common\Plugin`` interface.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will only be correct once we have released 1.1 right? we might want to add a versionadded block in strategically relevant places to explain what was changed in 1.1

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is versionadded?

Copy link
Contributor

@dbu dbu May 3, 2016 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And what would be the format? Since from the documentation point of view we haven't added anything, but changed. Would it be "Since client-common 1.1 Plugins are part of that package" or "Prior to client-common 1.1 it was a separate package"?

Copy link
Contributor

@dbu dbu May 3, 2016 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I will add it.

One more though: we probably have to always mention the package as well, since the versioning is not aligned between repos.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, good point. so lets be even more explicit and say something like

The plugins where moved to the clients-common package in version 1.1. If
you work with version 1.0, you need to require the separate plugin
package. The new interface will keep implementing
Http\Client\Plugin\Plugin until version 2 but relying on the old
interface is deprecated.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new interface will keep implementing

Hm, this is not true, and even if it was, it should be the other way around IMO: the old interface should extend the new one.

@dbu
Copy link
Contributor

dbu commented May 3, 2016

The new interface will keep implementing

Hm, this is not true, and even if it was, it should be the other way
around IMO: the old interface should extend the new one.

but then we have a BC problem i think. if i wrote code with the old interface, it should keep working until we go to 2.0. so the new interface should extend the old interface. or did we say for this edge case we do a BC break?

@sagikazarmark
Copy link
Member Author

Technically, there is no BC break. The "old" plugin client is still available in the old plugin package. If you want to use the old plugins, you still can with the old client.

What I could imagine is to extend the old plugin interface with the new one, so that old plugins work in the new client as well, but I don't think we have to maintain it vica versa (new plugins in the old client).

There is already kind of a PR for that: php-http/plugins#64

@dbu
Copy link
Contributor

dbu commented May 3, 2016 via email

@sagikazarmark
Copy link
Member Author

Yup. We might even support both in HttplugBundle?

@dbu
Copy link
Contributor

dbu commented May 4, 2016

i'd rather not add that overhead in the bundle.

can you add the versionadded blocks and then we merge this?

@sagikazarmark
Copy link
Member Author

Yep, working on it, also adding deprecation notice to plugins. It seems after all, that we might be able to have cross-compatibility between the new and the old client, plugins. PR coming.

@sagikazarmark
Copy link
Member Author

Done. Can you check @dbu?


.. versionadded:: 1.1
The plugins were moved to the clients-common package in version 1.1.
If you work with version 1.0, you need to require the separate `php-http/plugins` package.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you work with version 1.0, the interface is called Http\Client\Plugin\Plugin and you need to require the separate...

@sagikazarmark
Copy link
Member Author

Done

@dbu dbu changed the title WCM: Update plugin documentation Update plugin documentation May 4, 2016
@dbu dbu merged commit d4362be into master May 4, 2016
@dbu dbu deleted the plugin_relocation branch May 4, 2016 08:02
@dbu
Copy link
Contributor

dbu commented May 4, 2016

👍

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

2 participants