-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Add runfiles concept to documentation #28257
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: master
Are you sure you want to change the base?
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.
Code Review
This pull request adds valuable documentation for the runfiles concept, a long-requested feature. It provides examples for C++, Go, Python, and Shell, along with the necessary build configurations and source files to make them runnable. While the C++ and Go examples are correct, I've found critical errors in the Python and Shell examples that would prevent them from running as-is. The same errors are present in both the documentation and the example files. I've provided suggestions to correct these issues. With these fixes, this will be an excellent contribution.
20c1e78 to
6c4d158
Compare
lberki
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.
Thanks for the PR! I only have minor stylistic nits.
|
@lberki thanks for the review! I addressed your comments. PTAL. |
|
@lberki looks like some tests are using an old C++ toolchain without the |
|
Thanks and apologies for that space thing; for some reason, I didn't realize that there are newlines after. re: |
a3e6c31 to
7ed3272
Compare
Note that rules_go was already a transitive dependency.
|
Thanks! I assume this is now ready for review? (before I apply the "ready to merge" label) |
Yes indeed. I was only waiting for the test results before I planned to ping you again. |
|
Oh sorry, I didn't realize I jumped the gun :) click |
|
@mering Could you please make a change to Missing module docstring [missing-module-docstring] |
This has been requested for a long time and many projects have been documenting this downstream (for example Fuchsia).
Documenting this for C++, Go, Python, Shell as these are the languages we are using. Feel free to add additional languages in a follow-up.
I couldn't find a way to embed the example files directly in the documentation to ensure the examples can easily be tested, so I just copied the content in the MDX file.
The dependency to
rules_goalready existed transitively, so this is not newly added. Consider using a separateMODULE.bazelfile for theexamplesfolder in the future.Fixes #10022.