-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Ruff: Add and fix DTZ002 & DTZ011 + reorg some timezone components #12974
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
base: dev
Are you sure you want to change the base?
Conversation
617543f
to
3254474
Compare
1c41163
to
98e0b87
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am curious how this behaves with the DD_TIME_ZONE
env var
98e0b87
to
adc9d18
Compare
Thanks for pointing it out. I forgot about this env var. In my opinion, it is quite confusing to have multiple places for setting the time zone. During deeper analysis, I found some leftovers (addressed in #12995) that might simplify this PR. I was also trying to find the reason for the existence of In my opinion, the idea was nice and I support it in some way (to have more settings in I'm willing to drop all |
I do agree with this approach. It would be really nice to have the system setting working everywhere, but that would require a bit of a migration that could have some consequences if it didn't go perfectly. I'm definitely open to doing this in the future, but am not sure now is best time |
adc9d18
to
889b11b
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
889b11b
to
ee0beb1
Compare
Conflicts have been resolved. A maintainer will review the pull request shortly. |
ee0beb1
to
07c1517
Compare
Add and fix:
Move the fetching of timezone - to load the latest value