Re: [pacman-dev] [PATCH] Remove incorrect output when downloading only
On Mon, Jun 6, 2011 at 11:36 AM, Nagy Gabor ng...@bibl.u-szeged.hu wrote: Patch looks fine to me but some minor comments : 1) just a matter of taste, I would reduce the number of levels with if (prompt download) install = 1 else if prompt ... else ... 2) can we have downloadonly and prompt=0 ? do we want to print the warning in that case ? 3) that patch highlights that these 2 blocks are completely identical and could maybe be factored like that : if (install_ignorepkg(pkg)) install = 1 else ignored = 1 continue 4) Would not be better to leave the install = 1 decision to the front-end's callback function? I am not sure everyone wants that default here. I think I agree with Nagy here, or at least something doesn't feel quite right with the second part of this as much as the first part. For now I'll apply the first hunk to maint, and we'll get the second part sorted out soon enough. -Dan
Re: [pacman-dev] [PATCH] Remove incorrect output when downloading only
Allan McRae wrote:
diff --git a/lib/libalpm/deps.c b/lib/libalpm/deps.c
index c5fb92e..b03cd48 100644
--- a/lib/libalpm/deps.c
+++ b/lib/libalpm/deps.c
@@ -570,8 +570,12 @@ static pmpkg_t *resolvedep(pmdepend_t *dep, alpm_list_t
*dbs,
if(_alpm_pkg_should_ignore(pkg)) {
int install = 0;
if(prompt) {
- QUESTION(handle-trans,
PM_TRANS_CONV_INSTALL_IGNOREPKG, pkg,
- NULL, NULL, install);
+ if(handle-trans-flags
PM_TRANS_FLAG_DOWNLOADONLY) {
+ install = 1;
+ } else {
+ QUESTION(handle-trans,
PM_TRANS_CONV_INSTALL_IGNOREPKG, pkg,
+ NULL, NULL,
install);
+ }
} else {
_alpm_log(PM_LOG_WARNING, _(ignoring
package %s-%s\n), pkg-name, pkg-version);
}
@@ -592,8 +596,13 @@ static pmpkg_t *resolvedep(pmdepend_t *dep, alpm_list_t
*dbs,
if(_alpm_pkg_should_ignore(pkg)) {
int install = 0;
if(prompt) {
- QUESTION(handle-trans,
PM_TRANS_CONV_INSTALL_IGNOREPKG,
- pkg,
NULL, NULL, install);
+
+ if(handle-trans-flags
PM_TRANS_FLAG_DOWNLOADONLY) {
+ install = 1;
+ } else {
+ QUESTION(handle-trans,
PM_TRANS_CONV_INSTALL_IGNOREPKG, pkg,
+ NULL,
NULL, install);
+ }
} else {
_alpm_log(PM_LOG_WARNING,
_(ignoring package %s-%s\n), pkg-name, pkg-version);
}
--
1.7.5.4
Patch looks fine to me but some minor comments :
1) just a matter of taste, I would reduce the number of levels with
if (prompt download)
install = 1
else if prompt
...
else
...
2) can we have downloadonly and prompt=0 ? do we want to print the warning in
that case ?
3) that patch highlights that these 2 blocks are completely identical and could
maybe be factored like that :
if (install_ignorepkg(pkg))
install = 1
else
ignored = 1
continue
Re: [pacman-dev] [PATCH] Remove incorrect output when downloading only
Patch looks fine to me but some minor comments : 1) just a matter of taste, I would reduce the number of levels with if (prompt download) install = 1 else if prompt ... else ... 2) can we have downloadonly and prompt=0 ? do we want to print the warning in that case ? 3) that patch highlights that these 2 blocks are completely identical and could maybe be factored like that : if (install_ignorepkg(pkg)) install = 1 else ignored = 1 continue 4) Would not be better to leave the install = 1 decision to the front-end's callback function? I am not sure everyone wants that default here. NG -- SZTE Egyetemi Konyvtar - http://www.bibl.u-szeged.hu This message was sent using IMP: http://horde.org/imp/
