-
Notifications
You must be signed in to change notification settings - Fork 393
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 annotation support for transient crates. #2392
Conversation
991b359
to
a21c92c
Compare
@illicitonion I've done a complete rewrite of annotations according to your suggestions of inlining them into MODULE.bazel. |
I tested this PR on our current setup (still trying to move to
I'm not sure yet what the right fix would be though. EDIT: I tried to "stringify" those labels, but the produced JSON can't be parsed anymore by cargo-bazel, so I'm definitely doing something wrong here. |
This is needed for bazelbuild#2392 as starlark doesn't JSON-serialize labels.
#2394 should fix this |
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 LGTM, thanks! Just one comment around booleans, then I think we're good to go.
This is needed for bazelbuild#2392 as starlark doesn't JSON-serialize labels.
This is needed for bazelbuild#2392 as starlark doesn't JSON-serialize labels.
This is needed for #2392 as starlark doesn't JSON-serialize labels.
I've just rebased. There was also apparently some problems with using relative labels, as they were being evaluated relative to the wrong package. I've fixed it up now by turning them into strings. |
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 so much!
No description provided.