Skip to content

Conversation

odow
Copy link
Member

@odow odow commented May 14, 2021

I don't think we need to do this. I tried it out and it actually makes things slightly less readable, because the Variable and Constraint modules need to have MOIB.IndexInVector. It's a trivial type that isn't the source of our problems, and the difference isn't even really measurable.

@odow odow added the Submodule: Bridges About the Bridges submodule label May 14, 2021
@odow odow mentioned this pull request May 14, 2021
10 tasks
@blegat
Copy link
Member

blegat commented May 14, 2021

From withing the module, you need MOIB.IndexInVector instead of IndexInVector but from outside MOI, it replaces MOIB.Constraint.IndexInVector so it improves readability.
Some user might be confused using the wrong type and getting errors. Merging them should avoid this confusion.

@odow odow changed the title [bridges] remove unneeded TODO [bridges] unify IndexInVector May 14, 2021
@odow
Copy link
Member Author

odow commented May 14, 2021

Okay. Done.

@odow odow merged commit 90e0fe1 into master May 15, 2021
@odow odow deleted the odow-patch-1 branch May 15, 2021 02:50
@blegat blegat added this to the v0.10 milestone May 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Submodule: Bridges About the Bridges submodule
Development

Successfully merging this pull request may close these issues.

2 participants