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

Remove pytz dependency, update minimum python version to 3.9, extend tests #43

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

do3cc
Copy link

@do3cc do3cc commented Mar 10, 2025

This is the PR to implement #42
I've reviewed timezone/utils.py and noticed a lot of code isn´t used at all.
Then I added tests for windows and macos to make sure, nothing breaks.
Since nothing broke, I got worried and extended the tests to use timezones as provided by Python 3.9.
Then my windows tests failed and I added the necessary dependency for windows.
Now that I am writing this, I am wondering whether we should depend on tzinfo, for windows, as we don´t use the available timezones anywhere. However, I like that there are tests for that.

do3cc added 2 commits March 11, 2025 00:48
Also, force tests to use it, to check if it works with all OS
@justinmayer
Copy link
Member

Thanks for your work on this, Patrick.

@venthur / @hugovk: I am preparing for a trip and could use an extra pair of eyes. Would you mind taking a look and providing any feedback you may have?

@do3cc
Copy link
Author

do3cc commented Mar 11, 2025

After this I also downloaded pelican and made a test run with my modified branch. No failures. However I saw a few skipped tests due to missing locales.

do3cc and others added 2 commits March 11, 2025 10:39
Simplify config

Co-authored-by: Bastian Venthur <mail@venthur.de>
Copy link
Contributor

@hugovk hugovk left a comment

Choose a reason for hiding this comment

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

Also:

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 5b811c0..d2d7027 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,7 +1,7 @@
 ---
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.6.0
+    rev: v5.0.0
     hooks:
       - id: check-added-large-files
       - id: check-ast
@@ -15,7 +15,7 @@ repos:
       - id: trailing-whitespace
 
   - repo: https://github.com/asottile/pyupgrade
-    rev: v3.15.2
+    rev: v3.19.1
     hooks:
       - id: pyupgrade
-        args: [--py38-plus]
+        args: [--py39-plus]

@do3cc
Copy link
Author

do3cc commented Mar 12, 2025

Everything addressed. Thanks for the review

@venthur
Copy link
Collaborator

venthur commented Mar 13, 2025

Thanks for your contribution and addressing all requests @do3cc! @justinmayer the PR looks good to go.

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.

4 participants