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

Drop obsolete model_bakery.timezone.now #141

Merged
merged 3 commits into from
Feb 17, 2021
Merged

Conversation

amureki
Copy link
Collaborator

@amureki amureki commented Feb 10, 2021

I decided to do smaller steps to tackle #35 :)
It should be simpler to review-iterate now.

P.S. I wonder if this should be considered as a breaking change, but I doubt anyone who switched to model_bakery was using Django 1.4 and was importing this method.

P.P.S. Tests did fail due to the setup.py import issue. I put a separate PR for them: #142 and rebased this one on top of that branch. So the only meaningful here is the first commit: 2ec3eac. To simplify review, I changed base branch to about, which will be switched to main automatically after #142 would be merged.

@amureki amureki self-assigned this Feb 10, 2021
Rust Saiargaliev added 2 commits February 10, 2021 21:36
And use `django.utils.timezone.now` instead.
Refs #35.
@amureki amureki force-pushed the issues/35/drop_custom_now branch from d513fcf to 2ec3eac Compare February 10, 2021 20:43
@amureki amureki requested review from anapaulagomes and berinhard and removed request for anapaulagomes February 10, 2021 20:52
@amureki amureki changed the base branch from main to about February 11, 2021 15:45
@anapaulagomes anapaulagomes self-requested a review February 13, 2021 21:36
Copy link
Contributor

@anapaulagomes anapaulagomes left a comment

Choose a reason for hiding this comment

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

Cool! 🏆 If you think is worth it, we could also release a test version on Test Pypi and get feedback from the community.

@amureki
Copy link
Collaborator Author

amureki commented Feb 17, 2021

@anapaulagomes now I wonder if this would target the right audience here. :)
I don't think there is much sense in asking for active community members (that most likely are using latest or LTS versions of software) to test import that is obsolete since Django 1.4.

Also, we are not officially supporting it since long, haha, so I think this is just good to go as it is now.
Thank you very much for the review! 👍
I'll merge #142 and after that will merge this PR as well...

Base automatically changed from about to main February 17, 2021 16:48
Copy link
Member

@berinhard berinhard left a comment

Choose a reason for hiding this comment

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

@amureki this PR looks great! I'll ask you to rebase it from main because I've already merged #142 .

@berinhard
Copy link
Member

Feel free to merge this @amureki

@amureki amureki merged commit ac51be6 into main Feb 17, 2021
@amureki amureki deleted the issues/35/drop_custom_now branch February 17, 2021 21:43
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