Skip to content
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

A thorough modernization of the MOI wrapper #108

Merged
merged 4 commits into from
Feb 12, 2019
Merged

A thorough modernization of the MOI wrapper #108

merged 4 commits into from
Feb 12, 2019

Conversation

odow
Copy link
Member

@odow odow commented Feb 12, 2019

This PR closes all outstanding MOI related issues and gets Cbc in a nice shape for JuMP 0.19 release.

It was easier to just tidy up the wrapper in one shot that have many PRs sitting in CI.

Closes #107
Closes #106
Closes #104
Closes #102
Closes #96
Closes #81
Closes #79

@mlubin
Copy link
Member

mlubin commented Feb 12, 2019

Note the failure on 0.7 (it's ok to drop 0.7).

@mlubin
Copy link
Member

mlubin commented Feb 12, 2019

I didn't review in too much detail but it's hard to argue with the list of closed issues :). LGTM.
Isn't issue #102 already fixed by #105?

CC @vitornesello

@odow
Copy link
Member Author

odow commented Feb 12, 2019

The diff is hard to review in detail, but there were lots of bugs/unimplemented features like constants in the objective function. If this passes CI then it is passing every relevant MILP test in MOI so that seems pretty safe... Yay for a strong battery of tests.

@odow
Copy link
Member Author

odow commented Feb 12, 2019

I'll leave dropping 0.7 for another PR.

@mlubin mlubin merged commit f10e004 into master Feb 12, 2019
@mlubin mlubin deleted the od/modernize branch February 12, 2019 21:29
@mlubin
Copy link
Member

mlubin commented Feb 12, 2019

Tagged a new release as well.

@odow odow mentioned this pull request Feb 12, 2019
3 tasks
@vitornesello
Copy link
Contributor

Thanks guys, I took my time to read the code and indeed the code is better now.
Sorry for the absence lately. I have updated my notifications settings to not miss the messages mentioning me.

@odow
Copy link
Member Author

odow commented Feb 13, 2019

@vitornesello no worries! Thanks for already completing the bulk of the work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants