bpo-36043: FileCookieJar supports os.PathLike#11945
Conversation
tirkarthi
left a comment
tirkarthi
left a comment
There was a problem hiding this comment.
-
Docs should be updated https://docs.python.org/3.8/library/http.cookiejar.html#http.cookiejar.LWPCookieJar with versionchanged markup.
-
NEWS entry can use term:
path-like objectinstead of pathlib.Path and FileCookieJar can be linked with markup.
Thanks for the PR.
|
@tirkarthi done, could you review again? thanks |
|
|
||
| .. versionchanged:: 3.8 | ||
|
|
||
| The filename parameter supports a :class:`~pathlib.Path` instance. |
There was a problem hiding this comment.
As noted below this can be anything that implements fspath and not only pathlib.Path instance. Please use path-like object term with markup (:term:path-like object)
|
|
||
| .. versionchanged:: 3.8 | ||
|
|
||
| The filename parameter supports a :class:`~pathlib.Path` instance. |
There was a problem hiding this comment.
Please use path-like object as below
|
Out of curiosity, why inverting the |
|
@JulienPalard good catch, thank you, just updated with your recommendation. |
|
Because I stopped to raise an |
| filename = test.support.TESTFN | ||
| c = LWPCookieJar(filename) | ||
| self.assertEqual(c.filename, filename) | ||
| self.assertIsInstance(c.filename, str) |
There was a problem hiding this comment.
Are the type checks necessary? Unless the docs explicitly state that then I don't think they are necessary.
There was a problem hiding this comment.
no, they are not necessary, I have removed them. Thank you for your review.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase And if you don't make the requested changes, you will be poked with soft cushions! |
Co-Authored-By: matrixise <stephane@wirtel.be>
matrixise
changed the title
|
@brettcannon if you want to continue your review, thank you. |
|
@matrixise you forgot to ask Bedevere to mark this as ready to be reviewed again. 😄 Otherwise it doesn't end up in my review queue. |
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @brettcannon: please review the changes made to this pull request. |
|
useful this very small sentence for @bedevere-bot ;-) |
|
@matrixise thanks! |
7 participants
https://bugs.python.org/issue36043