Skip to content

Conversation

@HunkBenny
Copy link
Contributor

Using get_sols you can now retrieve multiple solutions

@HunkBenny
Copy link
Contributor Author

HunkBenny commented Jul 4, 2025

I am still actively learning, so I have two questions;

  1. Is adding a default implementation that panics (unimplemented()) a good thing to do?
  2. Is the failing cargo-clippy check something that I can solve?

@mmghannam
Copy link
Member

I am still actively learning, so I have two questions;

  1. Is adding a default implementation that panics (unimplemented()) a good thing to do?

Left you another comment :)

  1. Is the failing cargo-clippy check something that I can solve?

Seems like it's not liking the fact that russcip's rust edition is old. I wouldn't worry about it but I'll try to update it and see what happens 😄

@HunkBenny
Copy link
Contributor Author

You were correct! :)

I based myself on the implementation of conss. Seemed to have done the trick!

@mmghannam
Copy link
Member

You were correct! :)

I based myself on the implementation of conss. Seemed to have done the trick!

Great! just two more things:

  1. A test would be nice.
  2. Please update the version number in Cargo.toml, since the API changed the version has to increase. (0.8.3 should work)

@HunkBenny
Copy link
Contributor Author

I added a test for it! Also, it seems like updating the version number to 0.8.3 is not enough, I'll change it to 0.9.0 and commit it later today?

@mmghannam
Copy link
Member

Thank you very much @HunkBenny!

@mmghannam mmghannam merged commit eca9365 into scipopt:main Jul 9, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants