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

Filling out the curses stub #789

Closed
gotyaoi opened this issue Dec 23, 2016 · 4 comments · Fixed by #800
Closed

Filling out the curses stub #789

gotyaoi opened this issue Dec 23, 2016 · 4 comments · Fixed by #800

Comments

@gotyaoi
Copy link
Contributor

gotyaoi commented Dec 23, 2016

I'm doing a little work with curses recently, and noticed that the curses stub was all dynamic, so I gave a shot at filling some of it in. It needs a bit more work, but would this be useful to continue, and am I doing it correctly?

https://github.com/gotyaoi/typeshed/commit/e3dbef0714989d24e93b3161b8912555e7fe3325

@ambv
Copy link
Contributor

ambv commented Dec 23, 2016

Absolutely! Thank you for working on this. What you currently have already looks like a good improvement. You can cross-check your types with the library's documentation and Python's own unit tests for the library.

@gotyaoi
Copy link
Contributor Author

gotyaoi commented Dec 24, 2016

The tests didn't seem to assert much, so I just went by what was in _cursesmodule.c. https://github.com/gotyaoi/typeshed/commit/f5a88096fb421c26a7e374b4945eec3c08cc35d3

I cleaned up a few things, but I'm a little stuck on what to do about the window objects. They seem to fall under this issue: #24. I'm reasonably sure I could document them, but I don't know what to call the class...

@gotyaoi
Copy link
Contributor Author

gotyaoi commented Dec 28, 2016

Well, it's an improvement even without the window stub, so should I create a pull request for what I have in the meantime?

@ambv
Copy link
Contributor

ambv commented Dec 28, 2016

Agreed. Make a pull request with what you have. Add a comment about the windows stubs missing and that should be fine for now.

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 a pull request may close this issue.

2 participants