On Wed, Jul 25, 2007 at 05:55:32PM +0200, Florian Festi wrote: > Hi! Howdy
So I'll come out and admit that I haven't really looked over the code; I just glanced at it briefly but decided this was a good chance to get up on the soapbox about some general stuff... > depcheck Refactoring > ==================== > > Now that the git infrastructure is in place it is time to decide what to do > with my search/depcheck code. 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? 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. > 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. git has a number of facilities for helping with this: * the config option diff.color will show trailing whitespace on a 'git dif' before commit. * apply.whitespace can rip out trailing whitespace when applying patches from others * enabling the default pre-commit hook (chmod +x .git/hooks/pre-commit) will prevent you from commiting changesets with trailing whitespace by default. -James
pgpKZoXewfuX0.pgp
Description: PGP signature
_______________________________________________ Yum-devel mailing list Yum-devel@linux.duke.edu https://lists.dulug.duke.edu/mailman/listinfo/yum-devel
