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

tests(test_zoneinfo_as_timezone): skip if tzinfo is not available on … #135

Merged
merged 3 commits into from Mar 28, 2023

Conversation

5j9
Copy link
Contributor

@5j9 5j9 commented Mar 23, 2023

…windows

On Windows, zoneinfo relies on tzinfo and without it the tests fail with import error.

Also, rewrite pickle file loading.
Previously open was using a relative path, which means the path used to depend on the current working directory and could fail if tests were triggered from a wrong directory.
The new load_pickle function will handle that.

5j9 added 2 commits March 23, 2023 13:16
…windows

On Windows, zoneinfo relies on tzinfo and without it the tests fail with
import error.

Also, rewrite pickle file loading.
Previously `open` was using a relative path, which means the path
used to depend on the current working directory and could fail if
tests were triggered from a wrong directory.
The new load_pickle function will handle that.
import it as _, so that no name conflict occurs at line 750.
@slashmili
Copy link
Owner

@5j9 / @hramezani do you know how to fix the build in windows? https://ci.appveyor.com/project/slashmili/python-jalali

If we want to apply this changes, I'd rather to fix the appveyor as well.

@slashmili slashmili self-requested a review March 28, 2023 10:18
@5j9
Copy link
Contributor Author

5j9 commented Mar 28, 2023

@slashmili I just triggered an Appveyor build for this commit and it worked fine: https://ci.appveyor.com/project/5j9/python-jalali

I read somewhere that this particular error may occur if Appveyor's authorization to read your Github account has expired. You may need to revoke the Github access by going to https://ci.appveyor.com/authorizations and then reauthorize it.

@slashmili
Copy link
Owner

Thanks @5j9! It was indeed authorization issue!

Copy link
Owner

@slashmili slashmili left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@slashmili slashmili merged commit 8f79a04 into slashmili:main Mar 28, 2023
7 checks passed
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.

None yet

2 participants