-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
relax convert conditions #2839
relax convert conditions #2839
Conversation
dc816e8
to
b0a3432
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small versionchanged entry in the docs for this and a simple test addition for this behaviour would be nice to have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
@ankith26 I'll just change some convert tests to not create a window, is that OK? |
Yes, that would be cool |
Converted to a draft since it has some feedback left to resolve. Looks like a nice improvement! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to run the formatter on the changed file, but apart from that this PR LGTM. Thanks! 🎉
PS: we now have pre-commit
setup for handling the formatting
Can anybody please tell me why the CI is failing, I don't see any failing tests and just red marks |
Error code 3221225477 is a segfault, but I don’t know what segfaulted from the log |
There is no reason convert() has to wait for the display to be initialised. If a parameter with the target surface is provided, we can always convert.