-
Notifications
You must be signed in to change notification settings - Fork 243
Buck2 docs #1957
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
Buck2 docs #1957
Conversation
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.
Hey @cbarrete, thanks for putting this together!
Can you pull out the formatting changes into a separate PR? There are two things going on here, so I'd like to focus on the changes in each of them separately. The format PR should be quick to review and pull in :).
As for the build system documentation, I'm not sure if we're in a state where we're ready to publicly support things yet. We're still at a point where the versions of Pyrefly and Buck's prelude are still tightly coupled, and breaking/backward incompatible changes are happening relatively often. I also don't think we have the bandwidth to offer much support around this yet.
If we're adding this documentation, I'd like to make it clear that it's pretty unstable and will continue to be for a while, and that while feature requests and bug reports are welcome, support will likely be lower priority than other issues for a while.
Let me know your thoughts around this, and I'm very open to discussion if you disagree with my requests or have other questions/ideas.
website/docs/configuration.mdx
Outdated
| script](https://buck2.build/docs/bxl/) to get the necessary information about | ||
| the targets to typecheck. | ||
|
|
||
| The following optional configuration options are also supported: |
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.
Can you update the options below to follow the format used for other config options?
Something like the following would be preferred
#### Buck2
<description>
**`<option>`**
<description>
- Type: ...
- Default: ...
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.
I'd be happy to, but this is a more complex option than all others, because it has multiple variants, and its value is a product type.
I've done my best, let me know if you'd like me to tweak it further.
|
|
||
| `<arg-flag>` is either `--file` or `--target`, depending on the type of | ||
| `<arg>`, and `<arg>` is an absolute path to a file or a build system's target. | ||
|
|
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.
It may also be worth describing the JSON we consume. If you don't want to do it yourself, I can fill that in for you. If you do though, the structure is here.
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.
I've done what I could based on the sample value in a unit test I found.
Let me know if this is good enough.
There is a .prettierrc file in the website directory, but it was not consistently applied.
|
Thanks for the feedback! I've tried to address all your comments, let me know how it looks to you. Also, apologies for the awkward stacking, I'm not very used to branch-based code review tools. I've opened #1969 under this review for the formatting changes alone. I've also added a big disclaimer that this is not well supported yet. |
|
Awesome, this is looking great! I'll pull it in and start the review process internally. Thanks for working on this.
No worries, I also haven't worked with open source git stuff in a while, so I may ask for some things that might be unreasonable, and it's totally fine to push back if my requests are weird.
That was the big thing I wanted to see addressed before the documentation was added. I think this is looking great so far, so I'll see if anyone else on the team thinks there's anything that needs to change with this, and I'll try to get it merged in soon. |
|
@connernilsen has imported this pull request. If you are a Meta employee, you can view this in D90133880. |
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.
Review automatically exported from Phabricator review in Meta.
|
@connernilsen merged this pull request in 9ef4d84. |
Summary
This PR adds some documentation about integrating Pyrefly with Buck2.
It also cleans up some formatting based on the existing Prettier configuration.
Test Plan
This is just changing documentation, there is no functional impact.
I tried to run
test.py, butI didn't bother to try more since this PR makes no code changes.