refactor: generate schema code in build.rs
#83
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MingweiSamuel
changed the title
MingweiSamuel
changed the title
MingweiSamuel
force-pushed
the
buildrs
branch
3 times, most recently
from
e75a175 to
807889d
Compare
MingweiSamuel
changed the title
build.rs
MingweiSamuel
commented
Mar 18, 2025
MingweiSamuel
force-pushed
the
buildrs
branch
3 times, most recently
from
3d3cbf4 to
4ec87bd
Compare
conditional compilation optional `autoupdate` feature use feature instead of cfg to avoid recompiling everything wip fix include models/spec wip generate models.rs store generated rust in git update autogen feature deps grab specs from online fixes route everything except endpoints now cleanup codegen preserve schema map order cleanup meta.rs final codegen in build.rs cleanup endpoints codegen build and docs tidying document build info for clippy cfgs actually working now, oops cleanup bare urls for rustdoc newtype enum fix rephrase docs
There was a problem hiding this comment.
Pull Request Overview
This pull request refactors the schema code generation by moving it from JavaScript into Rust via the build script (build.rs). Key changes include integrating code generation logic into the Rust build process, migrating tests into production modules, and updating the Cargo.toml/build configuration to support autogeneration.
Reviewed Changes
Copilot reviewed 59 out of 60 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| riven/src/consts/game_type.rs | Tests now reside inline; removed separate test file for GameType |
| riven/src/consts/game_mode.rs | Removed manual game mode definitions in favor of generated code |
| riven/build/main.rs | New build script handling autogen behavior and config environment vars |
| riven/build/autogen/* | New modules handling schema parsing, enum generation, endpoints, etc. |
| riven/Cargo.toml and README.md | Updated configuration and documentation for the autogen feature |
| .cargo/config.toml | Build configuration now forces compilation of the autogen build script |
Files not reviewed (1)
- autogen_src.bash: Language not supported
Comments suppressed due to low confidence (1)
riven/build/autogen/spec.rs:285
- The error message in this assert statement is unclear; consider rephrasing it to clearly indicate that nested object schemas cannot be converted into a type name.
assert!(properties.is_none(), "Schema for is nested object, cannot turn into type name",);
MingweiSamuel
commented
Mar 27, 2025
| } => { | ||
| assert!( | ||
| properties.is_none(), | ||
| "Schema for is nested object, cannot turn into type name", |
Owner
Author
There was a problem hiding this comment.
"Schema is for nested object, cannot turn into type name",
Owner
Author
|
Maybe even splitting the generated code into a separate crate would be useful. To separate the compilation unit, and to repair our broken semantic versioning |
MingweiSamuel
force-pushed
the
v/2.x.x
branch
2 times, most recently
from
8e8fcae to
3afa333
Compare
Owner
Author
|
Make sure to handle #91 tournament regions |
Owner
Author
MingweiSamuel
force-pushed
the
v/2.x.x
branch
from
1ca4567 to
c8a9b13
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Instead of in javascript.
The the
riven_autogencfg option (forrustc) enables the build script. If unset, the build script is not even compiled, which ensures fast builds when Riven is used as a dependency. If set, the build script will be compiled but the actual behavior depends on theRIVEN_AUTOGEN_DEVMODEenvironment variable. If unset or set to"outdir"will generating files toOUT_DIRto be used by Riven (instead of the checked-insrcfiles). If set to"src", the build script will overwrite the files insrcdirectly (this is what./autogen_src.bashdoes). If set to"none", the build script will be built but will not generate any files.The build script will not generally re-run after the initial build, unless the
RIVEN_AUTOGEN_NONCEenvironment variable changes, or something else in the build configuration changes. For example, settingRIVEN_AUTOGEN_NONCEto the current date will trigger an update once a day, assuming the build configuration otherwise doesn't change: