James Bowes wrote:
On Wed, Jul 25, 2007 at 05:55:32PM +0200, Florian Festi wrote:
From my perspective there are only two minor
issues left:
* check if local rpms are detected correctly
* this is more or less a policy issue - do pkg objects have to be from
the existing pkg classes depending on their sources
* Check setAsDep() call in checkInstall
* I think the code is more correct than it was. May be someone wants to
speak up against that?
Ehm, I hope you don't allude you unit test comments on these issues. They
cannot be fixed by writint tests. We have to decide what is the "right
behaviour" (tm).
Some unit tests here would be a great way to prove that the code is more
correct. Write a case that fails with the old code, and passes with the
new code.
Really all of these changes to the depsolver related code would greatly
benefit from unit tests. A large set of tests that both the old and the
new depsolver pass would help to reduce the fears of major regressions.
This would have been helpful during the switch to headerless depsolving
as well. I'm not calling out Florian here, I think we all should be
writing these tests.
I fully agree with you that more tests would help a lot. Although it is no
longer as urgent as while there was no rpmlib check. If you look a bit more
in detail you'll see that I added some test cases but they cover only a few
lines of code.
I think in addition to unit tests we need a test suite consisting of a large
number of spec files and instructions what to do with them and the expected
results. Such test suite could be used to verify rpm, too (I know Panu is
looking forward to do some real development on rpm).
Main thing missing is a review for inclusion. Yeah, it's quite some code.
If someone wants to start I'd be glad to offer any help - be it about the
code, where to start or how to handle git. Any volunteers?
Regarding the code and git, I noticed that your patch series introduces
a lot of trailing whitespace. IMO this is like leaving in print lines,
or commented out code.
Can we remove them in one commit after the patches are merged, please. I
don't think it like the idea of merging all the patches a third time...
Florian
_______________________________________________
Yum-devel mailing list
Yum-devel@linux.duke.edu
https://lists.dulug.duke.edu/mailman/listinfo/yum-devel
