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

fix test build #144

Merged
merged 8 commits into from
Aug 29, 2023
Merged

fix test build #144

merged 8 commits into from
Aug 29, 2023

Conversation

cielavenir
Copy link
Contributor

@cielavenir cielavenir commented Aug 28, 2023

@cielavenir
Copy link
Contributor Author

cielavenir commented Aug 28, 2023

4d9d52d fixes macos, though 2.7 cannot be fixed due to package removal (Now how can I maintain my own projects...)

@benhoyt
Copy link
Owner

benhoyt commented Aug 28, 2023

Thanks for this! What do you think about trying to fix 2.7 at the same time, by separating out 2.7 into a separate job and using this technique?

@bschollnick
Copy link

Does this (now fixed) bug affect Python 3.10.2?

try --require-scandir-extension
@cielavenir cielavenir force-pushed the fixBuild branch 2 times, most recently from bd6b5aa to c55be81 Compare August 28, 2023 01:05
@cielavenir
Copy link
Contributor Author

@bschollnick sorry what bug?

@cielavenir
Copy link
Contributor Author

cielavenir commented Aug 28, 2023

Now c55be81 enforces extension.

https://github.com/benhoyt/scandir/actions/runs/5993519451/job/16254028573 had build failure but we did not notice without enforcing extension.

Now maybe it is better to exclude Windows pypy-2.7 from CI?

@cielavenir
Copy link
Contributor Author

@benhoyt Addressed

@cielavenir cielavenir changed the title WIP: fix test build fix test build Aug 28, 2023
@bschollnick
Copy link

The Mac OS reverting to python code, instead of the C code? I might of grabbed the wrong comment?

@cielavenir
Copy link
Contributor Author

The Mac OS reverting to python code

we did not test if that even happened, though currently we can see it is using C.

please note that you should use pip.

Copy link
Owner

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Really appreciate this, thanks! Just a few small changes requested, and I'm wondering if we can go back to an environment variable rather than using the (deprecated) --global-option flag.

.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
.github/workflows/tests.yml Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
Copy link
Owner

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

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

Excellent, thanks for these changes.

@benhoyt benhoyt merged commit e33140f into benhoyt:master Aug 29, 2023
11 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.

3 participants