Martin Betak has posted comments on this change. Change subject: frontend: Introducing oVirtJS / GWT wrapper projects ......................................................................
Patch Set 9: (3 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 4: export class Http { Line 5: Line 6: // Send HTTP request to remote server. Line 7: // Return `Promise` that resolves to HTTP response text. Line 8: // Implementation is based on `XMLHttpRequest` object. > Martin, do you think we should separate service API from its default (works Maybe in the future when we will have proper packaging for browser/node.js but we shouldn't complicate things at this stage. 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. 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. > We shouldn't take XMLHttpRequest as reference here. I believe Http.send() method should provide sufficient level of abstraction for us, that is enable sending JSON objects over http. I'm sure such SPI is trivially implementable in node.js (at worst using JSON.stringify internally). Regarding the comment on interface method (in the future when we will split interface and impl) it shouldn't probably mention the XHR. But then it should be a proper JSDoc :-) 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 { > Yes, ES6 class with only static methods brings the memories of Java :-) Not sure if we should go overboard with the desctructuring. It makes sense for public APIs but for these small utilities it may not be necessary. 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
