[pacman-dev] [PATCH] Remove trans->targets
Hi! Patch attached. Bye ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
Your commit message (after I wrapped at 76 chars): Remove trans->targets Its implementation was quite broken: * add_loadtarget() might have silently filtered out some targets (replacing older version...), while ->target was recorded by trans_loadtarget => confused add_commit due to list_count(trans->targets) * this was used in sync.c to determine whether a target is implicit or not; wrong: 'repo/pkg' [now we give warning always; imho this is acceptable, before this patch we silently removed user confirmed replacements too] * now remove001.py fails (imho this is the natural behavior) The patch looks fine, but a few comments. I struggle to get through your commit message. Code uses (), [], -> and ==>. English should not. I know it is not your first language, but can you please step away from the symbols when you write up your message? Example: Remove trans->targets The implementation was quite broken: * add_loadtarget() could possibly silently filter out some targets when it replaced an older version. <not even sure what the rest meant> When target was recorded by trans_loadtarget(), add_commit() was mislead due to the incorrect list_count(trans->targets) call. * The target list was used in sync.c to determine whether a target was implicit or not. This was incorrect behavior and we could silently remove user-confirmed replacements as well. * remove001.py was an odd test in that it installed the same package 5 times, this behavior is not necessary if the library does its job correctly so should be changed. I REALLY appreciate your work, but I would appreciate it even more if you could just simplify your commit writeup a bit. -Dan
On Mon, Mar 10, 2008 at 07:07:04PM -0500, Dan McGee wrote:
Your commit message (after I wrapped at 76 chars):
Remove trans->targets
Its implementation was quite broken: * add_loadtarget() might have silently filtered out some targets (replacing older version...), while ->target was recorded by trans_loadtarget => confused add_commit due to list_count(trans->targets) * this was used in sync.c to determine whether a target is implicit or not; wrong: 'repo/pkg' [now we give warning always; imho this is acceptable, before this patch we silently removed user confirmed replacements too] * now remove001.py fails (imho this is the natural behavior)
The patch looks fine, but a few comments. I struggle to get through your commit message. Code uses (), [], -> and ==>. English should not. I know it is not your first language, but can you please step away from the symbols when you write up your message? Example:
I agree that the form is a bit confusing :)
Remove trans->targets
The implementation was quite broken: * add_loadtarget() could possibly silently filter out some targets when it replaced an older version. <not even sure what the rest meant> When target was recorded by trans_loadtarget(), add_commit() was mislead due to the incorrect list_count(trans->targets) call.
Well, add_loadtarget silently skipped the target, which means it didn't add it to trans->packages, but trans_loadtarget still added it to trans->targets. That means that trans->targets and trans->packages were not in sync anymore. So add_commit should use list_count(trans->packages) rather than list_count(trans->targets).
* The target list was used in sync.c to determine whether a target was implicit or not. This was incorrect behavior
The wrong behavior wasn't clearly explained, only by the 'repo/pkg'. It seems that I indeed overlooked something. That usage of the targets list wasn't corect, because I forgot about the 'repo/pkg' syntax that could be used in the targets list, and didn't take this into account.
and we could silently remove user-confirmed replacements as well.
* remove001.py was an odd test in that it installed the same package 5 times, this behavior is not necessary if the library does its job correctly so should be changed.
I REALLY appreciate your work, but I would appreciate it even more if you could just simplify your commit writeup a bit.
-Dan
_______________________________________________ pacman-dev mailing list pacman-dev@archlinux.org http://archlinux.org/mailman/listinfo/pacman-dev
I REALLY appreciate your work, but I would appreciate it even more if you could just simplify your commit writeup a bit.
OK, the main reason: we doesn't need it at all;-) Bye ---------------------------------------------------- SZTE Egyetemi Könyvtár - http://www.bibl.u-szeged.hu This mail sent through IMP: http://horde.org/imp/
participants (3)
-
Dan McGee
-
Nagy Gabor
-
Xavier