Skip to content

various tweaks and cleanups #116

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

Merged
merged 1 commit into from
Jul 30, 2014
Merged

various tweaks and cleanups #116

merged 1 commit into from
Jul 30, 2014

Conversation

dbu
Copy link
Contributor

@dbu dbu commented Jul 29, 2014

decided to do a PR against your PR so you can see what i am doing.

@@ -460,10 +460,6 @@ private function addFlashMessageSection(ArrayNodeDefinition $rootNode)
->defaultFalse()
->info('Whether the cookie should only be transmitted over a secure HTTPS connection from the client.')
->end()
->scalarNode('httpOnly')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reading up on this again it simply makes no sense whatsoever to have a httpOnly cookie that is intended to be used in javascript. removed this option.

Copy link
Member

Choose a reason for hiding this comment

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

👍

@dbu
Copy link
Contributor Author

dbu commented Jul 29, 2014

@ddeboer can you have a look here and if you agree merge it into your branch?

@ddeboer ddeboer mentioned this pull request Jul 29, 2014
@@ -18,7 +18,7 @@ debug information when that header is present:
enabled
-------

**type**: ``enum`` **default**: ``auto`` **options**: ``true``, ``false``, ``auto``
**type**: ``enum``, **default**: ``auto``, **options**: ``true``, ``false``, ``auto``
Copy link
Member

Choose a reason for hiding this comment

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

Nitpicking: the Symfony docs have no commas here either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay. i just felt it looks bad with the readthedocs theme we have.

@dbu
Copy link
Contributor Author

dbu commented Jul 29, 2014

removed the commas. imho its good to merge now.

ddeboer added a commit that referenced this pull request Jul 30, 2014
@ddeboer ddeboer merged commit f2e8688 into refactor-docs Jul 30, 2014
@ddeboer ddeboer deleted the refactor-docs-dbu branch July 30, 2014 06:47
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.

2 participants