Skip to content

Replacement for #255#316

Merged
digitalresistor merged 6 commits into
masterfrom
pr/255
May 22, 2017
Merged

Replacement for #255#316
digitalresistor merged 6 commits into
masterfrom
pr/255

Conversation

@digitalresistor
Copy link
Copy Markdown
Member

Merging master into #255, and will add docs in this PR too.

Comment thread docs/experimental/samesite.txt Outdated
=================

The `Same-site cookie RFC
<https://tools.ietf.org/html/draft-west-first-party-cookies-07>`_ updates
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove extra space:

07>`_  updates`

Comment thread docs/experimental/samesite.txt Outdated
`RFC6265 <https://tools.ietf.org/html/rfc6265>`_ to include a new cookie
attribute named ``SameSite``.

WebOb provides support for setting the ``SameSite`` attribute in it's cookie
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

its

Comment thread docs/experimental/samesite.txt Outdated
attribute named ``SameSite``.

WebOb provides support for setting the ``SameSite`` attribute in it's cookie
API's, using the ``samesite`` keyword argument.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

APIs (neither possessive nor conjunction)

Comment thread webob/cookies.py Outdated
The name of the cookie.

``value``
The ``value`` of the cookie, if it is ``None`` it will generate a cookie
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The ``value`` of the cookie. If it is ``None``, it 

Comment thread webob/cookies.py Outdated
Default: ``None`` (browser scope).

``path``
The path used for the session cookie. Default: ``'/'``.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Default: /. (because we use a literal, no need to quote)

Comment thread webob/cookies.py
session cookie. Default: ``False``.

``samesite``
The 'SameSite' attribute of the cookie, can be either ``b"Strict"``,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ditto: Perhaps SameSite?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nope.

Comment thread webob/response.py
``samesite``

A string representing the ``SameSite`` attribute of the cookie or
``None``. If samesite is ``None`` no ``SameSite`` value will be sent
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are the various casing and markups of samesite and SameSite intentional?

Also are there two contexts or one for samesite/SameSite? Each context should use consistent markup.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes. This is on purpose. One is a kwarg, the other is the attribute name in the RFC.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, then should it be this for consistency and indicate that it is a kwarg?

 ``samesite``

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also you switch between:

'SameSite'

and

``SameSite``

Comment thread webob/response.py Outdated

A string representing the ``SameSite`` attribute of the cookie or
``None``. If samesite is ``None`` no ``SameSite`` value will be sent
in the cookie. Should only be ``b"Strict"`` or ``b"Lax"``
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add period to end of sentence.

Comment thread CHANGES.txt Outdated

These features are experimental and may change at any point in the future.

- The cookie API's now have the ability to set the SameSite attribute in both
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

APIs (this is neither possessive nor a conjunction)

Also see following comments about "SameSite" and "samesite".

Comment thread CHANGES.txt Outdated
These features are experimental and may change at any point in the future.

- The cookie API's now have the ability to set the SameSite attribute in both
webob.cookies.make_cookie and webob.cookies.CookieProfile.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should these be marked up with reST?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, this is in CHANGES.txt, I haven't marked any of the other things up either. I might add `` around them.

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