Omer Frenkel has posted comments on this change. Change subject: core: add template version fields to vm and template ......................................................................
Patch Set 8: (2 comments) http://gerrit.ovirt.org/#/c/22736/8/packaging/dbscripts/upgrade/03_04_0480_add_template_version_to_vm_static.sql File packaging/dbscripts/upgrade/03_04_0480_add_template_version_to_vm_static.sql: Line 1: select fn_db_add_column('vm_static', 'template_version_number', 'INTEGER DEFAULT NULL'); > naming: I think template_version is enough. its type is already an INTEGER regarding naming: it was template_version before, but since we have template_version and template_version name, it looked better to have the 'number' explicit in the name to avoid confusion (it was also asked by Alissa in comment 4) regarding default, i dont mind using 0 or -1, but in the code it will look like hardcoded magic number, which i like less: .. if (getVm().getTemplateVersionNumber == -1) { updateVersion(); } looks ok to you? Line 2: select fn_db_add_column('vm_static', 'template_version_name', 'varchar(40) DEFAULT NULL'); Line 3: -- make all existing vms and templates have version 1 Line 4: update vm_static set template_version_number=1; Line 5: -- update base template for all templates Line 1: select fn_db_add_column('vm_static', 'template_version_number', 'INTEGER DEFAULT NULL'); Line 2: select fn_db_add_column('vm_static', 'template_version_name', 'varchar(40) DEFAULT NULL'); > can't we just reuse the current template name and auto postfix it because user can already have this as an existing template name Line 3: -- make all existing vms and templates have version 1 Line 4: update vm_static set template_version_number=1; Line 5: -- update base template for all templates -- To view, visit http://gerrit.ovirt.org/22736 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I08f13eee5609525880a92fc81c9fd3cb1d2ba62d Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
