[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Triggers status?



On Wed, 24 Oct 2007, Ian Jackson wrote:
> Firstly, I had to merge my triggers branch with the latest head.  I've
> done this now and the result is in the same place as before:
>   http://www.chiark.greenend.org.uk/~ian/git/dpkg/dpkg.triggers/

Some more random remarks, this time on the content of the patches.

* you replaced a bunch of "NULL" by "(char*)0". This reverts 
  http://git.debian.org/?p=dpkg/dpkg.git;a=commit;h=4e5846ccd3dcc33504aba8ef35a8962bccfd562e
  I believe you should use NULL everywhere. (My personal taste also favors
  NULL over "(char*)0" but it's not even a question of what I prefer since
  I'm not an official dpkg maintainer)

* it would have been nice to have the various bits of refactoring somewhat
  separated (in other commits I mean) to simplify the review (it's still
  doable with "git rebase -i", selecting "edit" on commits to split, and
  then using "git add -i" to add subset of changes to the index and to
  commit them in several steps).
  I say this for the future but it's not a pre-requesite for further
  review (at least not on my part).

Direct patch review:

@@ -109,6 +109,7 @@ static void usage(void) {
 "  -E|--skip-same-version     Skip packages whose same version is installed.\n"
 "  -G|--refuse-downgrade      Skip packages with earlier version than installed.\n"
 "  -B|--auto-deconfigure      Install even if it would break some other package.\n"
+"  [--no-]triggers            Skip or force consequential trigger processing.\n"
 "  --no-debsig                Do not try to verify package signatures.\n"
 "  --no-act|--dry-run|--simulate\n"

Should logically be "--[no-]triggers".

No remarks on the core of the patch, as I'm not familiar enough with the
C codebase.

Cheers,
-- 
Raphaël Hertzog

Premier livre français sur Debian GNU/Linux :
http://www.ouaza.com/livre/admin-debian/



Reply to: