-
-
Notifications
You must be signed in to change notification settings - Fork 228
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
Do dependency resolution after checking valid configs #2960
Conversation
✅ PR OK, no changes in deprecations or warnings Total deprecations: 8 Total warnings: 0 Build statistics: statistics (-before, +after)
executable size=5297464 bin/dub
rough build time=62s Full build output
|
1435836
to
6378251
Compare
6378251
to
9cb8214
Compare
9cb8214
to
32220a6
Compare
Could you use the new testing framework ? If you need some help setting it up let me know. I can also try to write a test myself but I won't be able to get to it until end of next week. |
Sure, happy to help ^^ I'm just kinda out of the loop, so IDK what the new testing framework is 😅 Looking in the latest commits I'm assuming that it means to add it as a unit test on the I made a branch with that workflow here master...grillo-delmal:dub:unittest_issue_2695 by adding a couple of unittest to the dependencies.d file Tell me if I got it right or if it needs some changes (like using a new file or something) and we can continue from there ;). |
Correct.
That looks good! In both cases, you might want to enforce that the right dependency is selected (
I try to group the tests by category but it's a bit arbitrary and categories will likely evolve with time. Anything related to dependency resolution was added to |
32220a6
to
628d2b5
Compare
Ok, updated the tests and rebased the PR over 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.
Thanks!
This is my attempt to fix #2695
This PR changes the dependency resolver algorithm so that it marks which resulting config is going to be used after all the incompatible configs have been discarded.