Conversation
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more
tausbn
left a comment
tausbn
left a comment
There was a problem hiding this comment.
One comment, otherwise LGTM.
|
@criemen: ping 🙂 |
criemen
left a comment
criemen
left a comment
There was a problem hiding this comment.
LGTM! Annoying that we have to work around the upstream bug like this, but it works, so let's go ahead.
| @@ -149,11 +149,7 @@ fn write_schema( | |||
| .iter() | |||
| .map(|node| node_src_to_schema_class(node, &super_types)), | |||
| ); | |||
| // the concat dance is currently required by bazel | |||
| let template = mustache::compile_str(include_str!(concat!( | |||
| env!("CARGO_MANIFEST_DIR"), | |||
There was a problem hiding this comment.
So this is still not fixed :/
There was a problem hiding this comment.
what do you mean? The new code does not need to concat with the CARGO_MANIFEST_DIR env variable, so the new simpler code works both with cargo and with bazel.
There was a problem hiding this comment.
That rules_rust sets the proper variables here in line with cargo.
There was a problem hiding this comment.
ah, yes, seems like something went astray there, as it was working before but stopped working at some point. So not something required in this instance, but there could be a problem elsewhere.
rust/ast-generator/src/main.rs
Outdated
| @@ -558,10 +554,7 @@ fn write_extractor(grammar: &AstSrc) -> mustache::Result<String> { | |||
| nodes: grammar.nodes.iter().map(node_to_extractor_info).collect(), | |||
| }; | |||
| // the concat dance is currently required by bazel | |||
There was a problem hiding this comment.
agh, thought I had removed this comment
|
@criemen mind reapproving? |
3 participants
There's a bug in
rules_rustthat required a workaround.