-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactoring into submodules #18
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this first sketch of SOI!
I will make a more thorough review tomorrow, as you add lots of new functions inside SOI :)
# TODO This module should be replaced by the package https://github.com/JuliaStochOpt/StochOptInterface.jl | ||
module StochOptInterface | ||
|
||
using DocStringExtensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we continue to use DocStringExtensions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No we can get rid of it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reason to stop using it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am in a minimalistic mood for a few months ...
I tend to believe that the fewer the dependancies, the easier the maintenance is!
src/StructDualDynProg.jl
Outdated
include("solution.jl") | ||
# TODO This module should be replaced by the package https://github.com/JuliaStochOpt/StochOptInterface.jl | ||
include("StochOptInterface/StochOptInterface.jl") | ||
const SOI = StochOptInterface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the similitude with MOI!
Codecov Report
@@ Coverage Diff @@
## master #18 +/- ##
==========================================
+ Coverage 88.03% 88.31% +0.27%
==========================================
Files 14 17 +3
Lines 869 924 +55
==========================================
+ Hits 765 816 +51
- Misses 104 108 +4
Continue to review full report at Codecov.
|
No description provided.