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

use text gitattributes and context managers for tests #542

Closed
wants to merge 2 commits into from

Conversation

mattip
Copy link

@mattip mattip commented Aug 7, 2021

When trying to get the pypy windows pyyaml-feedstock to run cleanly, I needed to make these changes

  • add a .gitattributes file to tell git that the test data files are text (this was already in the feedstock but maybe not upstreamed?)
  • convert a few of the hanging open() into proper context managers so that the files were closed. This is important on PyPy + Windows when writing to files.

I got a bit carried away and converted all of them, as long as I was making a PR, even though most of them are read-only so it doesn't really matter.

@mattip
Copy link
Author

mattip commented Aug 8, 2021

This Pr is awaiting approval to run

@mattip
Copy link
Author

mattip commented Aug 11, 2021

This PR needs approval to run CI

@mattip
Copy link
Author

mattip commented Aug 12, 2021

The build failed for python2.7 on macos since cibuildwheel no longer supports python2.7. I pinned the cibuildwheel version. Each new changeset needs approval to run CI

@nitzmahone
Copy link
Member

PS: there's nothing special about our GHA- we're just on the normal free plan, so if you're still needing to debug issues, you can do that on your own fork so I don't have to keep pushing the button 😆

@nitzmahone
Copy link
Member

But yeah, I've hit the need to pin cibuildwheel in several places as well- I added the cibuildwheel version pin to the config matrix for cffi for the same reason...

@mattip
Copy link
Author

mattip commented Aug 12, 2021

Good idea. The last commit ran successfully in my fork.

@mattip
Copy link
Author

mattip commented Oct 15, 2021

This has too many conflicts with master, I will close it and try to redo

@mattip mattip closed this Oct 15, 2021
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.

2 participants