Fix bfbs Lua/Nim generators: honor -o and create namespace dirs #8765
Conversation
github-actions
bot
added
c++
codegen
beingbrijesh
force-pushed
the
pr/fix-bfbs-namespace-output
branch
from
7b8b8f7 to
3c33db7
Compare
beingbrijesh
force-pushed
the
pr/fix-bfbs-namespace-output
branch
from
3c33db7 to
8edf3bd
Compare
|
@cosmith-nvidia @dbaileychess @aardappel can you please review my fixes for the issue #8762 |
|
@beingbrijesh just a friendly note - if you update your description with "fixes #8762" it will link this PR and that issue :) |
@thejtshow thank you for your feedback, I have updated the description :) |
jtdavis777
left a comment
jtdavis777
left a comment
There was a problem hiding this comment.
I looked over the lua changes and test strapping, and it all looks good to me, though I'm no expert and I am not sure what the etiquette is around the goldens folder :)
I'm also not sure if the cmake based unit tests are executed as part of the CI -- that should probably be confirmed before merging. It seems like most tests are ran from dedicated shell scripts.
| std::replace(path.begin(), path.end(), '.', '/'); | ||
| } | ||
|
|
||
| // TODO(derekbailey): figure out a save file without depending on util.h |
There was a problem hiding this comment.
I think that the old TODO which was here is still relevant
| @@ -0,0 +1,8 @@ | |||
| # Test that bfbs generators handle namespace-first schemas correctly | |||
There was a problem hiding this comment.
I love this change since it integrates with IDEs better. Would you be able to add tests/py_test.py here as well?
|
This is not the way to do it, the It also generates new files (for Nim) without renaming the old ones. Generally, if something can be tested with the existing files in |
3 participants
"Fixes #8762 ": Use ConCatPathFileName(options_.output_path, path) and EnsureDirExists(full_dir) so generators create namespace directories under the provided -o path.
Also: added goldens/schema/namespace_first.fbs, updated Lua goldens, added tests/test_bfbs_namespace_output.py (Lua + Nim tests), and integrated the Python test into CMake.
Testing: built flatc, ran CTest; both new tests passed locally.