Embedded scripts cleanup - #1741
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the workspace utility scripts by moving the inline script strings from cleaner.py and file_editor.py into standalone files under share/ramble/scripts/. The workspace logic now copies these scripts directly using shutil.copyfile, and corresponding tests have been added. Feedback suggests using os.scandir() as a context manager in _ramble_cleaner.py to ensure proper resource cleanup, and defensively checking for None values when loading module specs in _ramble_file_editor.py.
Ramble Performance Test MetricsResults produced with commit: b2e0446
|
9dff05d to
776e222
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1741 +/- ##
========================================
Coverage 94.05% 94.05%
========================================
Files 369 370 +1
Lines 37180 37234 +54
========================================
+ Hits 34968 35022 +54
Misses 2212 2212 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
linsword13
left a comment
There was a problem hiding this comment.
Great stuff!
One nit question: are there other checks (like mypy) that should be enabled for these /share/ramble/scripts/** files?
|
Hmm great question. I will push something to enable mypy for those files |
Signed-off-by: Bob <robertbird@google.com>
Signed-off-by: Bob <robertbird@google.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Bob <robertbird@google.com>
Signed-off-by: Bob <robertbird@google.com>
Signed-off-by: Bob <robertbird@google.com>
b2e0446 to
4632b98
Compare
Previously we had python files (eg
_ramble_cleaner.py) hiding as comments which we then echo to file. This PR promotes them to real files and allows them to be copied insteadThis means we can do things like lint and reason about them more easily