James Bowes wrote:
So I played around with the ffesti branch/patch series tonight, and
everything looks good to me. I'd say this is ok to merge, with the
exception of the sqlite index workaround.
Yeah, I just put that in to avoid patching the metadata-parser all the time.
Nevertheless we can discuss if it makes sense to create all indexes on the
client side. Creating them is cheap (as long as you don't have an XO) and it
should save a significant percentage download size. But I don't have all
numbers at hand right now and we can consider it after we got the patches
merged. If we get the delta meta data code working this considerations are
obsolete anyway...
That said, I have a few minor nits, some of which are with the code
surrounding your changes, rather than your changes themselves. None of
these should stop your patches from being merged, IMO:
I tried to keep the size of the patch below 1000 lines so I couldn't do too
much additional cosmetics ;)=
* There are some decent sized chunks of commented out code that should
just be removed.
* We should replace the 'return 0' and 'return 1' with real booleans.
* It would be nice to have better names for get(Old|New)Provides and
Requires (I can't think of any myself, however).
Yeah, better name is welcome. If I'd know one myself we wouldn't discuss
that right now...
* Likewise for tsInfo.setDatabases (maybe make this two calls?)
I don't think it makes sense to set just one. A nicer interface would be to
pass the databases to the constructor but I wanted to avoid the interface
change to increase the change of getting the patches merged. Fell free to
adjust the code.
* 3 of the depsolve unit tests fail, but only because the fake rpmdb is
missing the new calls.
Ehmm, ok, I should have caught that one myself. Why don't we use a
PackageSack for this?
Nice work.
Thanks!
Thanks for taking care!
Florian
_______________________________________________
Yum-devel mailing list
[email protected]
https://lists.dulug.duke.edu/mailman/listinfo/yum-devel
