On Mon, Jun 6, 2011 at 11:36 AM, Nagy Gabor <[email protected]> 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
