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

Show Varnish configuration in tabs #131

Merged
merged 1 commit into from
Nov 14, 2014
Merged

Show Varnish configuration in tabs #131

merged 1 commit into from
Nov 14, 2014

Conversation

ddeboer
Copy link
Member

@ddeboer ddeboer commented Oct 28, 2014

Fix #130.

Thanks to @xabbuh we can now show config for different Varnish versions in configuration blocks: http://foshttpcache.readthedocs.org/en/doc-tabs/varnish-configuration.html#purge. I guess we need some custom CSS or Sphinx theme to actually render the <li>s as tabs.

@@ -29,7 +29,8 @@
"guzzle/plugin-mock": "*",
"mockery/mockery": "*",
"monolog/monolog": "*",
"symfony/process": "~2.3"
"symfony/process": "~2.3",
"fabpot/sphinx-php": "~1.0.6"
Copy link
Member Author

Choose a reason for hiding this comment

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

Either this or we ask devs to run pip install -r requirements. The requirements.txt we need anyway for building on ReadTheDocs.

Copy link
Contributor

Choose a reason for hiding this comment

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

lets go for "pip install". not everybody fixing a bug will want to build the doc i think.

@dbu
Copy link
Contributor

dbu commented Oct 29, 2014

great! i guess we can steal the css (and js?) snippet from the symfony docs page.

@ddeboer ddeboer changed the title [WIP] Show Varnish configuration in tabs Show Varnish configuration in tabs Nov 14, 2014
@ddeboer ddeboer force-pushed the doc-tabs branch 2 times, most recently from 0cbf292 to 434372a Compare November 14, 2014 14:05
@ddeboer
Copy link
Member Author

ddeboer commented Nov 14, 2014

Ok, this is now ready. Have a look at the URL above and merge if everything is fine.

@xabbuh
Copy link
Member

xabbuh commented Nov 14, 2014

At the Symfony documentation repository, we noticed that the Ruby highlighting doesn't fit the Varnish syntax well enough. Especially comments don't render as expected. Have a look at symfony/symfony-docs#4418 for what to change to use it instead.

@@ -39,6 +39,7 @@ before_script:

script:
- phpunit --coverage-clover=coverage.clover
- sudo pip install -r doc/requirements.txt
Copy link
Member

Choose a reason for hiding this comment

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

this should be part of the installation steps, not of the testing steps

@stof
Copy link
Member

stof commented Nov 14, 2014

I suggest putting the Varnish 4 tab first, and the Varnish 3 tab last. This way, the tab visible by default will be the config for the current Varnish version, not for the older one

@ddeboer
Copy link
Member Author

ddeboer commented Nov 14, 2014

@xabbuh Thanks, switched to CLexer.

@@ -88,7 +93,7 @@

# The theme to use for HTML and HTML Help pages. See the documentation for
# a list of builtin themes.
html_theme = 'default'
# html_theme = 'default'
Copy link
Contributor

Choose a reason for hiding this comment

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

what effect does commenting this out have ?

Copy link
Member

Choose a reason for hiding this comment

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

@dbu see the new config added on line 15

Copy link
Member

Choose a reason for hiding this comment

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

btw, it might be better to add it here rather than on line 15, to keep it near the comment explaining it

@ddeboer
Copy link
Member Author

ddeboer commented Nov 14, 2014

@stof Switched to Varnish 4 first.


Varnish 4
"""""""""
.. literalinclude:: ../tests/Functional/Fixtures/varnish-4/user_context.vcl
Copy link
Contributor

Choose a reason for hiding this comment

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

this seems the only vcl file starting with vcl 4.0; - this seemed not needed anywhere else. should we add it or remove it everywhere?

https://github.com/FriendsOfSymfony/FOSHttpCache/blob/master/tests/Functional/Fixtures/varnish-4/user_context.vcl#L1

(the things you start notice when you can compare the config in tabs :-)

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 other user_context*.vcls have it too. It’s because they are being used directly in the functional tests. In the other functional tests, fos.vcl is used (which has vcl 4.0 again), but then any other files are included, so they don’t require the header. I don’t think adding vcl 4.0 everywhere would make sense.

@dbu
Copy link
Contributor

dbu commented Nov 14, 2014

this looks really a LOT better! thanks @xabbuh and @ddeboer !

we might or might not look into the vcl 4.0; thing - feel free to merge david when the last version is rendered and you see no regressions.

Add sphinx-php to pip requirements for building on RTD

Rename ‘Testing the Library’ to ‘Contributing’

Add JS and CS for tabs

* Make syntax highlighting resemble that of Symfony docs more.

Remove fabpot/sphinx-php from require-dev

* Now in requirements.txt

Improve styling

Add pip install to Travis

Try to fix static path on RTD

Run pip install as sudo on Travis

Make border none important

Revert path to static

Move pip install to install steps

Clean up tabs.js

Remove sys.path.insert

Switch from RubyLexer to CLexer

Add link to Sphinx installation

Remove duplicated html_theme

Show Varnish 4 first
@ddeboer
Copy link
Member Author

ddeboer commented Nov 14, 2014

@dbu As explained above, we need the vcl 4.0 in user_context*.vcl, but I now hide it from the docs. Rebased and squashed, so feel free to merge when green.

@dbu
Copy link
Contributor

dbu commented Nov 14, 2014

awesome!

dbu added a commit that referenced this pull request Nov 14, 2014
Show Varnish configuration in tabs
@dbu dbu merged commit 9073dc9 into master Nov 14, 2014
@dbu dbu deleted the doc-tabs branch November 14, 2014 15:28
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.

update doc to have varnish config tabs
5 participants