Refactoring depsolving code for clarity #1367
Conversation
dralley
changed the title
dralley
changed the title
|
I filed this purely for tracking purposes https://pulp.plan.io/issues/4923 |
dralley
force-pushed
the
fix-modular-depsolv
branch
from
d4c1eda
to
1560035
Compare
This code is very difficult to follow. Here are some improvements that make it more comprehensible. * Add docstrings * Rename functions to have more descriptive names * Turn methods that do not need to be methods into functions * Separate the concerns of performing depsolving and loading data from the database re: #4693 https://pulp.plan.io/issues/4693 closes: #4923 https://pulp.plan.io/issues/4923
dralley
force-pushed
the
fix-modular-depsolv
branch
from
1560035
to
b2ecaa5
Compare
|
hi @dralley, can we run the tests related to RPM against this PR before we merge it? |
|
@kersommoura , yes. Jenkins ran the tests against this though, correct? It looks like they all passed. |
|
@dralley, jenkins runs unittests. What I meant is to run those tests locally https://github.com/PulpQE/Pulp-2-Tests/tree/master/pulp_2_tests/tests/rpm. The functional tests just run once the package with this PR is created, then if there is any failure we will need to investigate the root cause. |
|
@kersommoura Thanks for the correction. Sure we can wait for passing smash tests. Do you want me to do that or did you mean QE? |
There was a problem hiding this comment.
@dralley , thank you for refactor, it looks good to me.
I'm reviewing now not to block you later. Feel free to merge it if the smash tests locally pass.
|
@dralley, we currently do not have an easy way to install pulp 2 from a certain branch or PR, if you can run the tests locally that would be great. |
|
There were a couple of spurious test failures but none related to copy/association or modules. On previous times when I've run the tests I've also gotten those, so I'm assuming it's just an environment thing because these changes would not affect those parts whatsoever. Merging. |
This code is very difficult to follow. Here are some improvements that
make it more comprehensible.
the database
This does not fix any depsolving issues, but it will really help with maintaining and debugging this code.