On 17.12.2015 17:55, John Ferlan wrote: > As a follow on to a recent series which created macros for various > virsh-*.c command options, see: > > http://www.redhat.com/archives/libvir-list/2015-December/msg00672.html > > (although best to view by index since there are varying opinions on > the matter)... > > Modify virsh-domain.c and virsh-volume.c to "follow" the concept of > macro-ifying some commonly used options. In order to choose, I used: > > $ grep "{.name =" tools/virsh-*.c | \ > grep -v "help" | grep -v "desc" | grep -v NULL | \ > sort -i | uniq -c | sort -n > > To generate a list by module. That generated a list where I randomly > picked 9 as the nexus, leaving the following: > > 9 tools/virsh-domain.c: {.name = "persistent", > 10 tools/virsh-snapshot.c: {.name = "domain", > 12 tools/virsh-network.c: {.name = "network", > 12 tools/virsh-pool.c: {.name = "pool", > 13 tools/virsh-volume.c: {.name = "vol", > 14 tools/virsh-domain.c: {.name = "file", > 14 tools/virsh-domain-monitor.c: {.name = "domain", > 14 tools/virsh-volume.c: {.name = "pool", > 26 tools/virsh-domain.c: {.name = "current", > 28 tools/virsh-domain.c: {.name = "config", > 28 tools/virsh-domain.c: {.name = "live", > 81 tools/virsh-domain.c: {.name = "domain", > > Because virsh-domain-monitor.c, virsh-network.c, and virsh-snapshot.c > only had one element with larger counts - I just left them as is and > focused on the virsh-domain.c and virsh-volume.c. > > The perhaps more controversial choice will be the "file" option in > virsh-domain.c which had numerous different helpstr values. I chose > to macro-ify them using the entire helpstr including the N_("xxx") > rather than just "xxx" since the N_ is the I18N marker. > > John Ferlan (8): > virsh: Create macro for common "domain" option > virsh: Create macro for common "persistent" option > virsh: Create macro for common "config" option > virsh: Create macro for common "live" option > virsh: Create macro for common "current" option > virsh: Create macro for common "file" option > virsh: Create macros for common "pool" options > virsh: Create macros for common "vol" options > > tools/virsh-domain.c | 918 +++++++++++---------------------------------------- > tools/virsh-volume.c | 155 +++------ > 2 files changed, 247 insertions(+), 826 deletions(-) > Apart from Erik's suggestion, I wonder if we should put those definitions into a virsh-wide header so that we don't have to repeat them. For instance, plenty of commands in virsh-domain-monitor.c have --domain too. Other than that this looks very good. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list