)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":18893,"name":"Miles Gould","email":"mgould@redhat.com","username":"pozorvlak"},"change_message_id":"454a2edd77b70edec0f5fa26cde8ca5c9815c7fc","unresolved":false,"context_lines":[{"line_number":10,"context_line":"project."},{"line_number":11,"context_line":"- Eliminate the use of certain event handlers in the node-list"},{"line_number":12,"context_line":"and node-details controllers, and replace with linear promise"},{"line_number":13,"context_line":"chaning. I believe the use of promise chaining is more performant,"},{"line_number":14,"context_line":"and reduces code complexity."},{"line_number":15,"context_line":"- Provide more descriptive names for several functions"},{"line_number":16,"context_line":"- Add documentation for several functions"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ffe62b97_d999856e","line":13,"updated":"2017-03-27 16:59:03.000000000","message":"Typo: \"chaining\".\n\nHave you measured whether promise chaining is more performant? I find I am often surprised when I measure such things :-) At a more basic level, is this area of the code a performance problem?\n\nSuper-tiny nit: please indent lines 12-14 so the bullets are more obvious.","commit_id":"ac81202a7010977755ad7ce35116faf510ea2641"},{"author":{"_account_id":19380,"name":"Peter Piela","email":"ppiela@gmail.com","username":"ppiela"},"change_message_id":"89d5dc1542184396afb7c9d166a6964b4f07cde9","unresolved":false,"context_lines":[{"line_number":10,"context_line":"project."},{"line_number":11,"context_line":"- Eliminate the use of certain event handlers in the node-list"},{"line_number":12,"context_line":"and node-details controllers, and replace with linear promise"},{"line_number":13,"context_line":"chaning. I believe the use of promise chaining is more performant,"},{"line_number":14,"context_line":"and reduces code complexity."},{"line_number":15,"context_line":"- Provide more descriptive names for several functions"},{"line_number":16,"context_line":"- Add documentation for several functions"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ffe62b97_ce22d6a4","line":13,"in_reply_to":"ffe62b97_d999856e","updated":"2017-03-27 17:32:33.000000000","message":"Will fix typo and indent.\n\nI have not measured the performance improvement associated with the use of promise chaining. However, the change replaces event handlers that must be continually checked as events are generated with a callback that is associated with a specific action and called when that action is complete. Event handlers also require additional maintenance in that they must be destroyed when no longer required. In this case events also introduce unnecessary complexity to the debugging process.","commit_id":"ac81202a7010977755ad7ce35116faf510ea2641"}],"ironic_ui/static/dashboard/admin/ironic/create-port/create-port.service.js":[{"author":{"_account_id":18893,"name":"Miles Gould","email":"mgould@redhat.com","username":"pozorvlak"},"change_message_id":"454a2edd77b70edec0f5fa26cde8ca5c9815c7fc","unresolved":false,"context_lines":[{"line_number":39,"context_line":"     * @param {object} node - Node to which the port will be associated"},{"line_number":40,"context_line":"     * @return {promise} Object describing the created port"},{"line_number":41,"context_line":"     */"},{"line_number":42,"context_line":"    function createPort(node) {"},{"line_number":43,"context_line":"      var options \u003d {"},{"line_number":44,"context_line":"        controller: \u0027CreatePortController as ctrl\u0027,"},{"line_number":45,"context_line":"        backdrop: \u0027static\u0027,"}],"source_content_type":"text/javascript","patch_set":1,"id":"ffe62b97_192c8d00","line":42,"updated":"2017-03-27 16:59:03.000000000","message":"\"createPort\" doesn\u0027t really describe the behaviour of this function: \"createPortModal\", maybe? Same goes for other such functions.\n\nThese functions are all quite similar - can they be unified?","commit_id":"ac81202a7010977755ad7ce35116faf510ea2641"},{"author":{"_account_id":19380,"name":"Peter Piela","email":"ppiela@gmail.com","username":"ppiela"},"change_message_id":"89d5dc1542184396afb7c9d166a6964b4f07cde9","unresolved":false,"context_lines":[{"line_number":39,"context_line":"     * @param {object} node - Node to which the port will be associated"},{"line_number":40,"context_line":"     * @return {promise} Object describing the created port"},{"line_number":41,"context_line":"     */"},{"line_number":42,"context_line":"    function createPort(node) {"},{"line_number":43,"context_line":"      var options \u003d {"},{"line_number":44,"context_line":"        controller: \u0027CreatePortController as ctrl\u0027,"},{"line_number":45,"context_line":"        backdrop: \u0027static\u0027,"}],"source_content_type":"text/javascript","patch_set":1,"id":"ffe62b97_0e155e9c","line":42,"in_reply_to":"ffe62b97_192c8d00","updated":"2017-03-27 17:32:33.000000000","message":"I disagree. On successful completion of the function a port has been created (and returned). To a client the service is providing the capability to create a port; the fact that the creation process is manual and involves a modal dialog seems like an implementation detail that should be hidden. If there were alternative methods for creating ports you can ask how would those functions be named; which might suggest renaming this function to something like createPortInteractive (similar to what ou suggest), but going back to a function named \"modal\" does not make sense to me.","commit_id":"ac81202a7010977755ad7ce35116faf510ea2641"}],"ironic_ui/static/dashboard/admin/ironic/node-details/node-details.controller.js":[{"author":{"_account_id":18893,"name":"Miles Gould","email":"mgould@redhat.com","username":"pozorvlak"},"change_message_id":"454a2edd77b70edec0f5fa26cde8ca5c9815c7fc","unresolved":false,"context_lines":[{"line_number":247,"context_line":"     * @return {void}"},{"line_number":248,"context_line":"     */"},{"line_number":249,"context_line":"    function editPort(port) {"},{"line_number":250,"context_line":"      editPortService.modal(port, ctrl.node).then(function() {"},{"line_number":251,"context_line":"        ctrl.refresh();"},{"line_number":252,"context_line":"      });"},{"line_number":253,"context_line":"    }"}],"source_content_type":"text/javascript","patch_set":1,"id":"ffe62b97_99777dd5","side":"PARENT","line":250,"updated":"2017-03-27 16:59:03.000000000","message":"Honestly, I think this code was more readable when the method was called \"modal\".","commit_id":"38327e446f0f92b818099ed7c0202576910cd387"},{"author":{"_account_id":19380,"name":"Peter Piela","email":"ppiela@gmail.com","username":"ppiela"},"change_message_id":"89d5dc1542184396afb7c9d166a6964b4f07cde9","unresolved":false,"context_lines":[{"line_number":247,"context_line":"     * @return {void}"},{"line_number":248,"context_line":"     */"},{"line_number":249,"context_line":"    function editPort(port) {"},{"line_number":250,"context_line":"      editPortService.modal(port, ctrl.node).then(function() {"},{"line_number":251,"context_line":"        ctrl.refresh();"},{"line_number":252,"context_line":"      });"},{"line_number":253,"context_line":"    }"}],"source_content_type":"text/javascript","patch_set":1,"id":"ffe62b97_2ee02296","side":"PARENT","line":250,"in_reply_to":"ffe62b97_99777dd5","updated":"2017-03-27 17:32:33.000000000","message":"I have to disagree with that, but am open to better approaches.","commit_id":"38327e446f0f92b818099ed7c0202576910cd387"}]}