change build-time codegen to by-feature only #68
Conversation
| { | ||
| const DATASETS: &[&str] = &["log", "mesh", "minecraft_savedata", "mk48"]; | ||
| for &name in DATASETS.iter() { | ||
| // bebop_compile_dataset(name); |
There was a problem hiding this comment.
what's needed to enable bebop in this repo? i saw them posting publicity about their encoding, had a look at how the encoding actually works (it's just alright), and had a look at their repo (where they are making some WILD unsubstantiated claims about relative performance, even after they claim to have addressed those discrepancies. there's just no evidence that they have any interest in real numbers to back up their claims, lol
There was a problem hiding this comment.
#23 is the blocking issue, it looks like betwixt-labs/bebop#153 is what I ran into. It's not strictly a blocking issue, but the amount of work it would add is pretty significant.
There was a problem hiding this comment.
what's needed to enable bebop in this repo? i saw them posting publicity about their encoding, had a look at how the encoding actually works (it's just alright), and had a look at their repo (where they are making some WILD unsubstantiated claims about relative performance, even after they claim to have addressed those discrepancies. there's just no evidence that they have any interest in real numbers to back up their claims, lol
While not as exhaustive as this project, you can find our Rust benchmarks in the lab - and there are a multitude of independent benchrmarks in Go, C#, JavaScript, etc you can find across Github and Google.
There was a problem hiding this comment.
yeah it seems like bebop's rust library usability is not necessarily great in existing projects, or perhaps just has very poor ergonomics generally, and there appears to be little to no motivation to improve it from their project. i can only imagine this contributes to the dearth of real comparative benchmarks, something of a bellwether for an encoding library's quality in my experience
mumbleskates
force-pushed
the
regenerate-redux
branch
from
3d705e9
to
12111a1
Compare
|
ok well i got it to correctly complain when the codegen is out of date. whatever the CI is doing when it regenerates the code it's making like 6000 lines of diffs, not sure what the best course of action here is. |
|
the massive diff in the last commit there is actually because of line separators; the committed flatbuffers files have CRLF at the moment. it's possible that just changing git settings (always commit "\n") will make this a non-issue in windows |
mumbleskates
force-pushed
the
regenerate-redux
branch
2 times, most recently
from
9352cba
to
f1a3f4d
Compare
instead of requiring a feature to *not* perform codegen at build time, we only enable that codegen by specific features. this allows using committed generated code for protocol buffers, flatbuffers, and cap'n proto by default, while still allowing tests or developers to assert that the relevant generated code is up-to-date automatically.
djkoloski
force-pushed
the
regenerate-redux
branch
from
f1a3f4d
to
029d910
Compare
|
Re: generated files - I briefly considered having CI regenerate the files and check them in, but I think your approach is better. Having CI check in files makes things a lot more complex and the gain is very minimal. Thanks for the PR, this is a big improvement to managing codegen for the frameworks that aren't derive-based! |
|
you're welcome! yeah i still think the ideal way is to have CI/build system do all the codegen. but when there's no artifact caching service like bazel, and/or the codegen process is an obstacle to people who want to build the library, and/or your code review system has absolutely no way to reason about or show you diffs in generated code... it's really not that much worse to just check it in and make the CI do it exactly right and then yell at you when the code is out of sync :) |
instead of requiring a feature to not perform codegen at build time, we only enable that codegen by specific features. this allows using committed generated code for protocol buffers, flatbuffers, and cap'n proto by default, while still allowing tests or developers to assert that the relevant generated code is up-to-date automatically.
note: i'm actually not certain whether the CI change here is correct or completely desired, but it seems like the philosophically correct thing to do here. this is how we do it at $work, since github's workflows for diffing generated files are so terrible slash nonexistent (and the codegen situation, for this repo in particular, is so cowboy-styled that it probably is a good idea regardless. much better to just commit all the generated code when the alternative is to make everyone fetch exactly the right codegen tools every single time).