Version 1.20260723.0#588
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR bumps the library version and updates the SED instance API across C++, Python, and JavaScript bindings/tests by replacing the boolean “running” indicator with a richer status enum (Idle/Running/Paused).
Changes:
- Introduce
SedInstance::Statusand a newstatus()accessor in the C++ API and implementation. - Update Python/JS bindings and all related tests to use
statusinstead ofisRunning/is_running. - Adjust the JavaScript binding to expose
SedInstance.Statusin a nested location and update JS demo/test code accordingly.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| VERSION.txt | Bumps project version to 1.20260723.0. |
| src/api/libopencor/sedinstance.h | Adds SedInstance::Status and status(); removes isRunning() API. |
| src/sed/sedinstance.cpp | Implements status computation; removes isRunning() implementation. |
| src/sed/sedinstance_p.h | Updates PIMPL interface to use status() instead of isRunning(). |
| src/bindings/python/sed.cpp | Exposes SedInstance.Status + status property; removes is_running. |
| src/bindings/javascript/sed.cpp | Exposes SedInstance.Status + status property; removes isRunning and remaps enum location in Module. |
| tests/api/sed/instancetests.cpp | Migrates C++ API tests from isRunning() to status(). |
| tests/api/sed/concurrenttests.cpp | Migrates concurrent C++ tests from isRunning() to status(). |
| tests/bindings/python/test_sed_instance.py | Migrates Python tests from is_running to status. |
| tests/bindings/python/test_sed_concurrent.py | Migrates Python concurrent test from is_running to status. |
| tests/bindings/javascript/sed.instance.test.js | Migrates JS tests from isRunning to status. |
| tests/bindings/javascript/sed.concurrent.test.js | Migrates JS concurrent test from isRunning to status. |
| tests/bindings/javascript/res/res/libopencor.js | Updates JS example code from isRunning to status. |
| src/bindings/python/solver.cpp | Removes export_values() from several enums. |
| src/bindings/python/logger.cpp | Removes export_values() from Issue.Type enum. |
| src/bindings/python/file.cpp | Removes export_values() from File.Type enum. |
Comments suppressed due to low confidence (1)
src/api/libopencor/sedinstance.h:77
isRunning()was removed from the C++ API. Even with the newstatus()API, removing the boolean convenience method is a source-breaking change for downstream users; consider keeping it as a simple wrapper (optionally documenting it as a compatibility helper) so existing code can migrate incrementally.
/**
* @brief Return the status of this instance.
*
* Return the status of this instance.
*
* @return The status of this instance.
*/
Status status() const noexcept;
/**
* @brief Run all the tasks associated with this instance.
*
* Run all the tasks associated with this instance.
*
* @return The elapsed time in milliseconds.
*/
double run();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Indeed, we don't currently take advantage of it (in our tests, at least) and it could result in name collision, not to mention that standard Python enums don't do this.
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
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.
No description provided.