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

Docs for new cache plugin option #179

Merged
merged 1 commit into from
Feb 28, 2017

Conversation

acrobat
Copy link
Contributor

@acrobat acrobat commented Feb 27, 2017

This PR adds the docs for the new cache plugin option introduced in php-http/cache-plugin#26.

Closes: #178

@acrobat acrobat force-pushed the respect-respons-header-docs branch 4 times, most recently from 500f1d6 to 0f71038 Compare February 27, 2017 11:53
Since v1.3.0 does the ``CachePlugin`` have 2 factory methods to easily setup the plugin by caching type.

.. code-block:: php
$cachePlugin = CachePlugin::clientCache($pool, $streamFactory, $options); // Which allows caching responses with the ``private`` or ``no-store`` cache directive
Copy link
Member

Choose a reason for hiding this comment

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

Can one have code blocks within notes?

Anyhow: make sure to split comments and code so the lines are not too long.

// comment
$cachePlugin = // ...

// comment
$cachePlugin = // ...


.. code-block:: php

$cachePlugin = CachePlugin::clientCache($pool, $streamFactory, $options); // Which allows caching responses with the 'private' or 'no-store' cache directive
Copy link
Member

Choose a reason for hiding this comment

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

Can one have code blocks within notes?

Anyhow: make sure to split comments and code so the lines are not too long.

// comment
$cachePlugin = // ...

// comment
$cachePlugin = // ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if it works as the preview screen on github doesn't show the output good. I will change the how the comments are placed!

I could also put a not and then in a separate code block (outside of the note) the example code?

@acrobat acrobat force-pushed the respect-respons-header-docs branch 2 times, most recently from 1f3ffe9 to 663fdb4 Compare February 27, 2017 11:58
@acrobat
Copy link
Contributor Author

acrobat commented Feb 27, 2017

@Nyholm I've fixed the comment about the line length and moved the code block out of the note (just to be sure it renders correctly!)

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

thanks for the quick PR. have some small requests for improvement.

@@ -125,7 +141,9 @@ to include them. You can specify any uppercase request method which conforms to
Cache Control Handling
----------------------

This plugin does not cache responses with ``no-store`` or ``private`` instructions.
By default this plugin does not cache responses with ``no-store`` or ``private`` instructions. If you need to cache
Copy link
Contributor

Choose a reason for hiding this comment

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

please add no-cache to this list. then say that with the clientCache factory method, no-store and private will be ignored.

.. note::

Since v1.3.0 does the ``CachePlugin`` have 2 factory methods to easily setup the plugin by caching type. See the
example below.
Copy link
Contributor

Choose a reason for hiding this comment

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

i'd suggest to move most of this out of the node and instead say just that there are the factory methods for the client resp. server mode. then have the code block, and then below a note just saying "The two factory methods have been added in version 1.3.0."

@acrobat
Copy link
Contributor Author

acrobat commented Feb 27, 2017

Ok, I've fixed your comments @dbu!

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

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

looks good to me

@Nyholm Nyholm merged commit de46ff8 into php-http:master Feb 28, 2017
@Nyholm
Copy link
Member

Nyholm commented Feb 28, 2017

Thank you!

@acrobat acrobat deleted the respect-respons-header-docs branch February 28, 2017 07:52
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