-
Notifications
You must be signed in to change notification settings - Fork 177
fix docstrings on functions written in C #1446
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
base: main
Are you sure you want to change the base?
Conversation
|
The CI now takes too long... |
kinto0
left a comment
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.
this is awesome. thank you for looking into this and finding a simple solution that I imagine works really well.
- it's unclear from the doc-comment if this script is executed on the client or on CI (bundled with the pyrefly binary). can you document it and explain the limitations? from the code, it seems like it's only run on CI.
- let's try removing that type: ignore. what made you add it (there's no pyrefly.toml)? if this crate is checked with a checker, can we vendor docify to avoid it? or does docify have a bunch of dependencies?
|
How this script works: It only runs on CI, writes the doc to stub, then bundles with the Pyrefly binary. The type ignore is ... because this script needs docify is a single script that relies on libcst. |
Co-authored-by: Kyle Into <[email protected]>
|
awesome! hit ready for review when you're ready, I'm excited to get this in. one thing I will change is add a buck target for the build script so we can run it through buck (and maybe remove the lint suppression?) |
yangdanny97
left a comment
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.
IMO better to run this bundling process whenever we pull in updates for the typeshed stubs, as opposed to every time we build. I believe that is what ty does as well.
That already happens using a Python script, so you could add your docstring copying logic to that directly.
| # This source code is licensed under the MIT license found in the |
fix #1076
use https://github.com/DetachHead/basedpyright/blob/main/build/py3_8/generate_docstubs.py to gen code during build and bundle into lsp.