Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
add rule for bindgen (#102) #108
add rule for bindgen (#102) #108
Changes from 12 commits
f096c8c
566f2fe
ebca9c3
a120a99
bf76c50
40e5d52
7209c28
8054608
ecb07c0
c1baedb
725501c
5070b0a
65c10d4
2534cd5
4bbcb7a
f558d5e
651627f
e824e0d
45592dd
485becc
25c3ca9
11616c3
243aa62
dfe5ded
20fd058
55f0075
6c0b35b
0e7c86e
f1f91e3
22ab34a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Is running rustfmt needed on generated sources? I meant, they are not made for being read anyway.
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 personally found it useful (in my own hacky version of this) when inspecting generated files. In my experience, bindgen can be a bit finnicky and require special params to configure generation. Its much easier to diagnose issues in a formatted file compared to a super long single line unformatted file.
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.
Beyond being useful, this is what bindgen does by default. I think it is best to keep parity with other tools whenever possible.
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.
Sounds good, can you just add a comment about it?
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 am surprised you did not got hit by bazelbuild/bazel#6889. This will get shipped in the host configuration :(
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.
Not sure if @mfarrugi fully follows the issue, but I'm a bit confused.
Isn't host configuration the configuration we want to run bindgen in?
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.
We want to run this at build-time on the host, so I don't follow what "shipped" means. (I also don't understand the issue :))
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.
If we want to run this in host configuration please add cfg = "host". The default value of cfg is "target" but due to bazelbuild/bazel#6889 it somehow becomes the host configuration. This is a bug in Bazel.