Skip to content

Conversation

shadiakiki1986
Copy link
Contributor

Related to #441

I followed the instructions in issue 441 (by odow) to create this PR + added 2 changes

  • travis badge links updated to travis-ci.com instead of travis-ci.org (currently the badge goes to an extra redirect page)
  • added a new page in the docs about testing. I think this needs some more revision, but I just wanted to get your opinion first about having such a page in the docs. It's basically the same instructions as odow laid out in the issue. I think that it'd be a good documentation for how the testing works (I personally wasn't expecting that the optimizers ran tests from the MOI package), as well as tests being tested, and where to plug in stuff in order to add a new test. Let me know if you think it's a bad idea.

I also tested that the ECOS tests would give out an error from the solve_twice if the early return in the optimize! is commented out (source).

Copy link
Member

@odow odow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work. To make it so we can merge some bits quicker, this could be split into three PRs.

  1. The change to the README.
  2. The new tests.
  3. The new documentation.

Ideally, we keep PR's as small and self-contained as possible.

@shadiakiki1986 shadiakiki1986 force-pushed the sa/issue441_doubleoptimize branch 2 times, most recently from 798c161 to 53d9d7c Compare November 4, 2020 08:07
@shadiakiki1986
Copy link
Contributor Author

Ok, I dropped the readme and new testing docs page from this PR

@shadiakiki1986 shadiakiki1986 force-pushed the sa/issue441_doubleoptimize branch from 53d9d7c to 596d32e Compare November 5, 2020 02:55
@shadiakiki1986
Copy link
Contributor Author

Rebased branch on upstream master

@odow odow merged commit dee7876 into jump-dev:master Nov 26, 2020
@shadiakiki1986 shadiakiki1986 deleted the sa/issue441_doubleoptimize branch November 26, 2020 03:22
@blegat blegat added this to the v0.9.19 milestone Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants