[Rpm-maint] [PATCH] re-factoring rpmInstall & rpmErase - merging common code & removing redundancy

[Panu Matilainen]

On Thu, 15 Jan 2009, Rakesh Pandit wrote:

> Hello list,

Hi,

Way past overdue to get your patches reviewed and merged, sorry to keep 
you waiting...

> Well holidays made me lazy and some other stuff kept me busy and could
> not update my earlier mails.
>
> Attached are updated coherent patches(updated all earlier mailed
> patches). I have fixed earlier patches to make them pass test suite
> and few cosmetic changes. My aim was refactoring of rpmInstall &
> rpmErase and removing any symmetric algorithm code between them. The
> basic aim being to implement last in ideas page[1] i.e "conflicting
> updates".
>
> I have kept them as small as possible, though first two may look long
> but that is just because of merging and corresponding renaming of
> structure members.
>
> First patch[1]:
>
> - Merge rpmInstallInterfaceFlags_e and rpmEraseInterfaceFlags_e to
> rpmCliInterfaceFlags_e.
> - Renamed (UN)?INSTALL_* to RPMCLI_* and all corresponding changes in
> different interface related files.

As we've talked (outside list), finding a good name for these things seems 
to be almost the hardest part here, silly as it is. The problem with "cli" 
for these flags is that it's *too* generic.

How about just merge rpmInstallInterfaceFlags_e and 
rpmEraseInterfaceFlags_e to rpmInstallFlags_e? Obviously erasing isn't 
installing, but it's at least in the ballpark.

>
> Second patch[2]:
>
> - Merged installInterfaceFlags and eraseInterfaceFlags into cliInterfaceFlags.

..and the same here, if we just follow what query verify etc do here, just 
call it 'rpmcliInstallFlags'.

> - Moved transaction part from rpmInstall and rpmErase to rpmCliTransaction.

I'd suggest leaving rpmCliTransaction() static for now, just to have the 
freedom of changing it's arguments if that becomes necessary later on. As 
long as rpmqv.c is using rpmInstall() and rpmErase(), rpmCliTransaction() 
doesn't need exporting.

Oh and rpmcliTransaction() (vs rpmCli...) would be more in line with the 
other cli-related things like rpmcliQuery(), doesn't of course matter if 
static.

> Third patch [3]:
>
> -Remove stopinstall variable and moved rpmcliPackagesTotal to correct place.
> -Changes in rpmInstallSource for getting reused in install mode and removed

Looks ok to me.

 	- Panu -