Martin Betak has posted comments on this change. Change subject: frontend: Introducing oVirtJS / GWT wrapper projects ......................................................................
Patch Set 9: (2 comments) https://gerrit.ovirt.org/#/c/33720/9/frontend/ovirtjs/ovirtjs-lib/src/main/services.js File frontend/ovirtjs/ovirtjs-lib/src/main/services.js: Line 8: // Implementation is based on `XMLHttpRequest` object. Line 9: // - method: HTTP method to use, defaults to `GET`. Line 10: // - url: URL to send request to, defaults to empty string (document base URI). Line 11: // - headers: HTTP headers, defaults to empty object. Line 12: // - body: Any value supported by `XMLHttpRequest.send` function, defaults to null. > Current oVirtJS code calls Http.send method with "body" parameter being eit Good point, this could then simplify (or even get rid of altogether) the transformHttpBody. I believe really converting the "object" into whatever format required by the transport layer is the responsibility of the Http service. As for the JSON.stringify and "other environments" I believe you exlicitly stated that oVirt.js will require for run an EcmaScript 5+ compliant environment and JSON.stringify is a part of the ES 5 specification. Really given that JSON.stringify is supported on anything from IE 8 to Node.js I don't imagine a reason to define another SPI just to convert to/from JSON :-) @JSDoc: Of course the tooling may not be there yet, but I think we should follow best practices from start - even when we won't be generating any html docs from that. Even in java code we use javadoc because is standard format and not because we really wanted an html output. Line 13: static send ({ method, url, headers, body }) { Line 14: return new Promise((resolve, reject) => { Line 15: var xhr = new XMLHttpRequest(); Line 16: https://gerrit.ovirt.org/#/c/33720/9/frontend/ovirtjs/ovirtjs-lib/src/main/utils.js File frontend/ovirtjs/ovirtjs-lib/src/main/utils.js: Line 1: // Utility for working with objects. Line 2: export class ObjectUtil { > Well, the main reason for using it would be consistency across all methods That is a good point. I don't know where exactly to draw the line and consistency is probably the best default. I guess, in this case I would support both implementations. Line 3: Line 4: // Compute change between `original` and `modified` objects. Line 5: // Return diff object containing the difference. Line 6: // Diff object will be null if there is no change. -- To view, visit https://gerrit.ovirt.org/33720 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2646646bc880446aa3035dd951e65c9af81933a1 Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vojtech Szocs <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Einav Cohen <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Greg Sheremeta <[email protected]> Gerrit-Reviewer: Jenny Kang <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Martin Betak <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: Zuzana Svetlíková <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
