Localize datetimes to defaults timezone #3632
Conversation
|
I think I've finished - I cannot find any more test to do. |
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thanks for the PR! The changes look good so far. We'll need a few newunit tests though :)
- tests for
extract_tzinfo_from_defaultsintest_datetime.py, where you pass atg.Bot, atg.ext.ExtBotwithout defaults and atg.ext.ExtBotwith defaults to ensure that everything works as expected - For all the changed classes we need ensure that the datetime objects are localized as expected in the three cases described above. For that, please add a
test_de_json_localization(or sim) after the respectivede_jsontests. The test fixtures linked in #3595 should be used for that.
Please do reach out if you need help with the unit tests - I know that those are not always simple :)
Bibo-Joshi
left a comment
There was a problem hiding this comment.
Thank you for the updates - LGTM! What a nice PR :) If we get another approval (maybe lemontree or pool), we can merge
Poolitzer
left a comment
There was a problem hiding this comment.
Missed a few changestrings.
Btw, is the docstring important for the args documentation? It only applies to the attributes of the classes, no...
|
@Poolitzer do you have any comments left? @Trifase I guess there is notheng to be added from your side? |
I think I fullfilled everything that was requested and everything else that was missing! |
|
Thank you very much for this nice contribution @Trifase ! I hope that you also had some fun with the PR :) |
When done, will close #3595
Checklist for PRs
.. versionadded:: version,.. versionchanged:: versionor.. deprecated:: versionto the docstrings for user facing changes (for methods/class descriptions, arguments and attributes)AUTHORS.rst(optional)