-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fixed doc for framework.session.cookie_lifetime refrence. #3475
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
Fixed doc for framework.session.cookie_lifetime refrence. #3475
Conversation
tyomo4ka
commented
Jan 14, 2014
Q | A |
---|---|
Doc fix? | yes |
New docs? | no |
Applies to | 2.3+ |
Fixed tickets |
``0``, which means the cookie is valid for the length of the browser session. | ||
This determines the lifetime of the session - in seconds. It will use ``null`` by | ||
default which means ``session.cookie_lifetime`` value from ``php.ini`` will be used. | ||
``0`` means the cookie is valid for the length of the browser session. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would use something like:
Setting this value to ``0`` means the cookie is valid for the length of the browser session.
It feels somewhat unnatural in the form it is now, but maybe I'm just too picky ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just too picky ;)
No, why? :) Your variant sounds better for me too.
Just one thing - as it is a fix for the 2.3. version you should request to merge it with the 2.3. branch not the master branch. |
@mtrojanowski Oops :) Sure. Will close this PR and open new. I can't change base branch for PR on GH. Am I wrong? |
No, you can't. But I guess @weaverryan can safely cherry pick this small commit. |
This determines the lifetime of the session - in seconds. By default it will use | ||
``0``, which means the cookie is valid for the length of the browser session. | ||
This determines the lifetime of the session - in seconds. It will use ``null`` by | ||
default which means ``session.cookie_lifetime`` value from ``php.ini`` will be used. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default, which means [...]
@wouterj Updated. Can you cherry-pick it in 2.3 branch. Or it is better to close this PR and open new based on 2.3? @mtrojanowski @xabbuh Thanks for notices. This is my first attempt to contribute to doc BTW ;) |
@tyomo4ka just leave this PR as it is now. We just said it so you know how to do it correctly the next time. Btw, I'm not the merger for the docs, that's @weaverryan :) |
Hey Artem! |
Hey Artem! This is actually awesome! Nice work on going through the code to find the subtle change that was needed here. I've merged this into the 2.3 branch - I hope to see more PR's from you :). Cheers! |
…tyomo4ka) This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #3475). Discussion ---------- Fixed doc for framework.session.cookie_lifetime refrence. | Q | A | ------------ | --- | Doc fix? | yes | New docs? | no | Applies to | 2.3+ | Fixed tickets | Commits ------- 39a65e1 Fixed doc for framework.session.cookie_lifetime refrence.