Eliminate duplicated BAE_STRICT_MODE parsing logic #8
Conversation
…RICT_MODE Co-authored-by: nosredna123 <[email protected]>
Co-authored-by: nosredna123 <[email protected]>
Co-authored-by: nosredna123 <[email protected]>
|
@copilot solve the code conflicts |
I've resolved the merge conflicts by rebasing onto Conflicts resolved:
Result:
The branch needs a force-push to update the remote since history was rewritten during rebase. The conflicts are fully resolved and the code is ready. |
|
@copilot reanalyze the PR, and resolve any code conflicts |
There was a problem hiding this comment.
Pull request overview
This PR eliminates duplicated BAE_STRICT_MODE parsing logic by centralizing configuration access through the Config class. The refactoring removes redundant inline environment variable parsing in favor of using the existing Config.BAE_STRICT_MODE property, improving code maintainability and consistency.
Key changes:
- Removed redundant
strict_modevariable assignments that duplicated the parsing logic already present in Config class - Updated code to directly reference
Config.BAE_STRICT_MODEinstead of parsingos.getenv("BAE_STRICT_MODE")inline - Enhanced test to verify Config class correctly parses BAE_STRICT_MODE environment variable
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
baes/core/enhanced_runtime_kernel.py |
Removed duplicate BAE_STRICT_MODE parsing logic in two locations (lines 1190 and 1258), now using Config.BAE_STRICT_MODE directly |
tests/unit/core/test_force_accept_mode.py |
Added importlib import and updated test_config_strict_mode_parsing to verify Config class parsing behavior by reloading the config module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| importlib.reload(config) | ||
| Config = getattr(config, 'Config') | ||
| assert Config.BAE_STRICT_MODE == True, f"Config.BAE_STRICT_MODE should be True for BAE_STRICT_MODE={value}" |
There was a problem hiding this comment.
After the test_config_strict_mode_parsing method completes, the config module will remain in the state from the last iteration (BAE_STRICT_MODE="off"). This could affect subsequent tests in the test suite. The test should restore the original config state after all iterations complete, either by reloading the config module with the original BAE_STRICT_MODE value or by storing and restoring the original Config.BAE_STRICT_MODE value at the end of the method.
| strict_mode = os.getenv("BAE_STRICT_MODE", "false").lower() in ("true", "1", "yes", "on") | ||
| assert strict_mode == True, f"Should be True for BAE_STRICT_MODE={value}" | ||
| importlib.reload(config) | ||
| Config = getattr(config, 'Config') |
There was a problem hiding this comment.
Using getattr(config, 'Config') to access the Config class is unnecessarily indirect. After reloading the config module, you can directly access config.Config instead of using getattr. This would make the code more readable and maintainable. The same applies to line 259.
| strict_mode = os.getenv("BAE_STRICT_MODE", "false").lower() in ("true", "1", "yes", "on") | ||
| assert strict_mode == False, f"Should be False for BAE_STRICT_MODE={value}" | ||
| importlib.reload(config) | ||
| Config = getattr(config, 'Config') |
There was a problem hiding this comment.
Using getattr(config, 'Config') to access the Config class is unnecessarily indirect. After reloading the config module, you can directly access config.Config instead of using getattr. This would make the code more readable and maintainable.
I've reanalyzed and successfully resolved all code conflicts by rebasing onto the latest Conflicts resolved:
Final changes:
Test results: ✅ All 6 tests in Commits: f1523de, e1fb68e, a1adced The branch has been rebased and is ready. It requires a force-push to update the remote since history was rewritten during conflict resolution. The conflicts are fully resolved and all tests pass. |
strict_modevariable)strict_mode = Config.BAE_STRICT_MODEin two locations in enhanced_runtime_kernel.pyConfig.BAE_STRICT_MODEdirectly in if statementsimport importlibfor test helper functionRebased onto: origin/pr-5 at commit 0608bb8 (includes merged PRs #6, #7, and #9)
Changes in this PR:
Config.BAE_STRICT_MODEin a localstrict_modevariableConfig.BAE_STRICT_MODEdirectly in conditional checks✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.