Skip to content
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

Timezone aware #253

Merged
merged 5 commits into from
Jan 7, 2013
Merged

Timezone aware #253

merged 5 commits into from
Jan 7, 2013

Conversation

duduklein
Copy link
Contributor

I made the timezone aware issue compatible with django 1.3

Basically, I try to import django.utils.timezone and when it fails, it falls back to datetime.datetime

I had to import timedelta directly when needed, i.e., from datetime import timedelta in order to use datetime as before.

I also made the use of dateutil optional. It if's not present, it will fall back to strptime. What I don't really like about this, is that the timezone is hardcoded (+0000). dateutil will detect the correct timezone.

I would like to add some tests to make sure that all the importerrors fall back to the original state, but I could not find a way to impose the ImportError (like using mock library). Any ideas on how to do this?

@tschellenbach
Copy link
Owner

Yeay raising import errors is quite hard. You can replace the builtin import function, but that isn't so nice.

If the import is done at the function and not the module level it is easy to raise using the side_effect attribute of the Mock.

The code isn't pep8 at the moment.
$ pep8 --exclude=migrations --ignore=E501,E225 django_facebook open_facebook
django_facebook/utils.py:2:56: W291 trailing whitespace
django_facebook/utils.py:458:1: E302 expected 2 blank lines, found 1
django_facebook/utils.py:459:59: W292 no newline at end of file

Other than that looks good though, merging :)

tschellenbach added a commit that referenced this pull request Jan 7, 2013
@tschellenbach tschellenbach merged commit 830d59e into tschellenbach:master Jan 7, 2013
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