regebro / tzlocal

A Python module that tries to figure out what your local timezone is
MIT License
184 stars 58 forks source link

Modernized the packaging / testing configuration #94

Closed agronholm closed 3 years ago

agronholm commented 3 years ago
agronholm commented 3 years ago

I don't know why the workflow was not triggered.

pganssle commented 3 years ago

I don't know why the workflow was not triggered.

I suspect that you cannot add new workflows that are triggered unless you have write access to the repository? I don't know the exact rules, but it's common to restrict this sort of thing so that people don't open thousands of PRs that run bitcoin miners or whatever on random unmaintained OSS projects.

agronholm commented 3 years ago

@regebro do you want to move tzlocal to use the src layout? I can modify this PR accordingly.

regebro commented 3 years ago

No, I prefer this layout, unless there is some new reason against it.

agronholm commented 3 years ago

The reason is that running the tests in the project directory will import the project source rather than the installed version of the package. That in turn means that the test suite won't expose any mishaps in the packaging (like missing files in MANIFEST.in). Moving the source to src/ prevents that from happening. I've switched to the src layout in all my recent projects and am in the process of migrating older projects to that as well as I update them.

agronholm commented 3 years ago

Looks like the workflow is running now, and failing on Windows.

regebro commented 3 years ago

Yeah, I wonder what the windows time zone is in github actions.

agronholm commented 3 years ago

Well, the traceback says Etc/GMT. Go figure.

agronholm commented 3 years ago

Oh, could this be caused by the missing tzdata?

agronholm commented 3 years ago

I added tzdata in #100, but that is now failing with a different error: https://github.com/regebro/tzlocal/pull/100/checks?check_run_id=1176719892

agronholm commented 3 years ago

Oh, it looks like a unix specific test. It should probably be skipped on Windows, yes?