[pacman-dev] [PATCH] Remove trans->targets

Dan McGee dpmcgee at gmail.com
Mon Mar 10 20:07:04 EDT 2008


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




More information about the pacman-dev mailing list