-
Notifications
You must be signed in to change notification settings - Fork 23
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
Add priority option for repo configuration #84
Conversation
Currently there's no user-specified ordering for the defined repositories and because of the way the code works it will (counter intuitively) choose RPMs from repos later in the list. This change introduces an explicit repo priority so that users can force ordering of repos to choose RPMs from.
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.
lgtm over all, do you think you could add a small unit test?
Now that we have the loading code exposed in a way that is a bit more manageable we can add some tests around it. We also add a set of helpers in a separate source file which will be used by tests for the reducer itself.
This change approximates the reducer operations and provides tests for them. It leverages the existing helpers produced for the loader_test to reduce the repetitiveness of constructing the test inputs.
The first iteration of this change produced a scenario where we'd drop packages that have a different version which we previously were retaining. This change fixes that and includes differently versioned packages while accounting for the repository priority otherwise.
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.
LGTM, I think this together with my stable solver setup will give the results we expect.
Let's wait a few days see if @rmohr has some time to look at it, otherwise I'll approve
Order makes sense, I just want to add that it should eventually use the newest versions matching a constraint, but yes if the same versions are shipped by two repos it just uses one of those kind of arbitrary. |
Currently there's no user-specified ordering for the defined repositories and because of the way the code works it will (counter intuitively) choose RPMs from repos later in the list.
This change introduces an explicit repo priority so that users can force ordering of repos to choose RPMs from.