Michael Kublin has posted comments on this change.
Change subject: engine: Additional clean up of OvfParser
......................................................................
Patch Set 1: (8 inline comments)
All remarks are wrong, actually all 9 remarks.
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfParser.java
Line 34: String id1 = "1";
Line 35: String id2 = "2";
Line 36:
Line 37: XmlNode node =
_document.SelectSingleNode("//*/Content/TemplateId");
Line 38: if (!StringUtils.isBlank(node.InnerText)) {
Why? "\t\n" points that xml is template?
Line 39: id1 = node.InnerText;
Line 40: }
Line 41:
Line 42: XmlNodeList list =
_document.SelectNodes("//*/Content/Section");
Line 42: XmlNodeList list =
_document.SelectNodes("//*/Content/Section");
Line 43: for (XmlNode section : list) {
Line 44: String value =
section.Attributes.get("xsi:type").getValue();
Line 45:
Line 46: if (StringUtils.equals(value,
"ovf:OperatingSystemSection_Type")) {
You can send a patch and replace all constants all over a code, or you can also
send a different patch on every constant.
Line 47: id2 = section.Attributes.get("ovf:id").getValue();
Line 48: }
Line 49: }
Line 50:
Line 64: return retVal;
Line 65: }
Line 66:
Line 67: public static Guid GetImageGrupIdFromImageFile(String imageFile) {
Line 68: if (!StringUtils.isBlank(imageFile)) {
Why, "\t\n" can be converted to Guid?
Line 69: return Guid.createGuidFromString(imageFile.split("[/]",
-1)[0]);
Line 70: }
Line 71: return null;
Line 72: }
Line 87: * @param str
Line 88: * @return the date or null if parse failed
Line 89: */
Line 90: public static Date UtcDateStringToLocaDate(String str) {
Line 91: if (StringUtils.isBlank(str)) {
So, I suppose "\t\n" can be parsed to Date in some formats or we need to catch
two exception before a null will be return?
Line 92: return null;
Line 93: }
Line 94:
Line 95: try {
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfVmWriter.java
Line 46: _writer.WriteEndElement();
Line 47: _writer.WriteStartElement("Origin");
Line 48: _writer.WriteRaw(String.valueOf(_vm.getOrigin().getValue()));
Line 49: _writer.WriteEndElement();
Line 50: if (!StringUtils.isBlank(_vm.getAppList())) {
No, you are wrong. I am not familiar with application with name "\t\n"
Line 51: _writer.WriteStartElement("app_list");
Line 52: _writer.WriteRaw(_vm.getAppList());
Line 53: _writer.WriteEndElement();
Line 54: }
Line 83:
Line 84: @Override
Line 85: protected void WriteAppList() {
Line 86: if (_images.size() > 0) {
Line 87: if (StringUtils.isBlank(_images.get(0).getappList())) {
same here
Line 88: return;
Line 89: }
Line 90:
Line 91: String[] apps = _images.get(0).getappList().split("[,]",
-1);
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfWriter.java
Line 216: _writer.WriteStartElement("initrd_url");
Line 217: _writer.WriteRaw(vmBase.getInitrdUrl());
Line 218: _writer.WriteEndElement();
Line 219: }
Line 220: if (!StringUtils.isBlank(vmBase.getKernelUrl())) {
Same here
Line 221: _writer.WriteStartElement("kernel_url");
Line 222: _writer.WriteRaw(vmBase.getKernelUrl());
Line 223: _writer.WriteEndElement();
Line 224: }
Line 221: _writer.WriteStartElement("kernel_url");
Line 222: _writer.WriteRaw(vmBase.getKernelUrl());
Line 223: _writer.WriteEndElement();
Line 224: }
Line 225: if (!StringUtils.isBlank(vmBase.getKernelParams())) {
Same here
Line 226: _writer.WriteStartElement("kernel_params");
Line 227: _writer.WriteRaw(vmBase.getKernelParams());
Line 228: _writer.WriteEndElement();
Line 229: }
--
To view, visit http://gerrit.ovirt.org/11240
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1c2e2d0a455694a4f9aa39e82f370d2085062019
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Michael Kublin <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
