launchpad-reviewers team mailing list archive

[Bayard 'kit' Randel <kit.randel@xxxxxxxxxxxxx>]

Bayard 'kit' Randel has proposed merging lp:~blr/launchpad/inline-comment-delint into lp:launchpad.

Requested reviews:
  William Grant (wgrant): code

For more details, see:
https://code.launchpad.net/~blr/launchpad/inline-comment-delint/+merge/243322

A delinting pass over the inline comment code I've been looking at recently.
-- 
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/code/javascript/branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js	2014-11-27 22:27:28 +0000
+++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js	2014-12-01 19:25:34 +0000
@@ -43,7 +43,7 @@
 namespace.flush_drafts_to_server = function() {
     var config = {
         on: {
-            success: function () {
+            success: function() {
                 Y.fire('inlinecomment.UPDATED');
             }
         },
@@ -89,10 +89,10 @@
         });
         widget.editor.on('keydown', function(e) {
            var enterKeyCode = 13;
-           if (e.domEvent.ctrlKey === true && 
+           if (e.domEvent.ctrlKey === true &&
                e.domEvent.button === enterKeyCode) {
                this.save();
-           } 
+           }
         });
         widget.editor.on('cancel', function(e) {
             handling_request = false;
@@ -111,7 +111,6 @@
           .append('<div class="editorShortcutTip">' +
                   '[Esc] Cancel, [Ctrl+Enter] Save' +
                   '</div>');
-
     };
     var delete_handler = function(e) {
         var line_number = namespace.find_line_number(e.currentTarget);
@@ -143,11 +142,14 @@
 
 namespace.create_row = function(comment) {
     // Find or create the container for this line's comments.
-    var comments_tr = Y.one('#comments-diff-line-' + comment.line_number);
+    var comments_tr = Y.one('#comments-diff-line-' +
+                            comment.line_number),
+        comment_date;
+
     if (comments_tr === null) {
-        var comments_tr = Y.Node.create(
-            '<tr class="inline-comments">'
-            + '<td colspan="2"><div></div></td></tr>')
+        comments_tr = Y.Node.create(
+            '<tr class="inline-comments">' +
+            '<td colspan="2"><div></div></td></tr>')
             .set('id', 'comments-diff-line-' + comment.line_number);
         Y.one('#diff-line-' + comment.line_number)
             .insert(comments_tr, 'after');
@@ -181,7 +183,7 @@
         // Wrap resources coming from LP.cache, as they come from
         // the LP API (updates during the page life-span). This way
         // the handling code can be unified.
-        if (LP.links['me'] !== undefined) {
+        if (LP.links.me !== undefined) {
             newrow.append('<div class="boardCommentFooter"></div>');
             newrow.one('.boardCommentFooter')
                 .append('<a class="js-action draft-edit">Reply</a>');
@@ -198,10 +200,10 @@
         // XXX cprov 20140226: the date should be formatted according to
         // the user locale/timezone similar to fmt:displaydate.
         // We should leverage Y.Intl features.
-        if (typeof comment.date === "string") {
-            var comment_date = Y.lp.app.date.parse_date(comment.date);
+        if (typeof comment.date === 'string') {
+            comment_date = Y.lp.app.date.parse_date(comment.date);
         } else {
-            var comment_date = comment.date;
+            comment_date = comment.date;
         }
         var date = Y.lp.app.date.approximatedate(comment_date);
         headerspan.one('span').set('text', date);
@@ -228,11 +230,11 @@
 namespace.populate_comments = function() {
     var config_published = {
         on: {
-            success: function (comments) {
+            success: function(comments) {
                 // Display published inline comments.
                 // [{'line_number': <lineno>, 'person': <IPerson>,
                 //   'text': <comment>, 'date': <timestamp>}, ...]
-                comments.forEach( function (comment) {
+                comments.forEach(function(comment) {
                     namespace.create_row(comment);
                 });
                 Y.fire('inlinecomment.UPDATED');
@@ -249,14 +251,14 @@
 namespace.populate_drafts = function() {
     var config_draft = {
         on: {
-            success: function (drafts) {
+            success: function(drafts) {
                 namespace.inlinecomments = {};
                 if (drafts === null) {
                     return;
                 }
                 // Display draft inline comments:
                 // {'<line_number>':''<comment>', ...})
-                Object.keys(drafts).forEach( function (line_number) {
+                Object.keys(drafts).forEach(function(line_number) {
                     var comment = {
                         'line_number': line_number,
                         'text': drafts[line_number]
@@ -284,7 +286,7 @@
     // loaded for anonymous requests.
     // In fact, if loading draft is attempted is will fail due to
     // the LP permission checks.
-    if (LP.links['me'] !== undefined) {
+    if (LP.links.me !== undefined) {
         // Add the double-click handler for each row in the diff. This needs
         // to be done first since populating existing published and draft
         // comments may add more rows.
@@ -295,7 +297,7 @@
 };
 
 
-var PublishDrafts = function () {
+var PublishDrafts = function() {
     PublishDrafts.superclass.constructor.apply(this, arguments);
 };
 
@@ -320,29 +322,31 @@
      * @method syncUI
      */
     syncUI: function() {
+        var n_drafts = Y.all('.draft').size(),
+            rc_scroller = Y.one('.review-comment-scroller'),
+            scroller_text = '';
+
         this.get('contentBox').empty();
-        var n_drafts = Y.all('.draft').size();
-        var rc_scroller = Y.one('.review-comment-scroller');
         if (rc_scroller !== null) {
             if (n_drafts > 0) {
-                var scroller_text = 'Return to save comment'
-                if (n_drafts > 1) scroller_text += 's';
+                scroller_text = 'Return to save comment';
+                if (n_drafts > 1) { scroller_text += 's'; }
                 rc_scroller.set('text', scroller_text);
             } else {
                 rc_scroller.set('text', 'Return to add comment');
             }
         }
-        if (n_drafts == 0) {
+        if (n_drafts === 0) {
             Y.fire('CodeReviewComment.SET_DISABLED', true);
             return;
         }
         var text = ' Include ' + n_drafts + ' diff comment';
-        if (n_drafts > 1) text += 's';
+        if (n_drafts > 1) { text += 's'; }
         var checkbox = Y.Node.create(
             '<input id="field.publish_inline_comments"' +
             'name="field.publish_inline_comments"' +
             'type="checkbox" class="checkboxType"' +
-            'checked=""></input>')
+            'checked=""></input>');
         var label = Y.Node.create(
             '<label for="field.publish_inline_comments" />')
             .set('text', text);
@@ -367,7 +371,7 @@
 namespace.PublishDrafts = PublishDrafts;
 
 
-var InlineCommentToggle = function () {
+var InlineCommentToggle = function() {
     InlineCommentToggle.superclass.constructor.apply(this, arguments);
 };
 
@@ -408,7 +412,7 @@
         if (display) {
             css_display = 'table-row';
         }
-        Y.all('.inline-comments').each( function() {
+        Y.all('.inline-comments').each(function() {
             this.setStyle('display', css_display);
             this.next().setStyle('display', css_display);
         });
@@ -469,12 +473,12 @@
      * @method initializer
      * @protected
      */
-    initializer: function(cfg){
+    initializer: function(cfg) {
         // If we have not been provided with a Launchpad Client, then
         // create one now:
-        if (null === this.get("lp_client")){
+        if (null === this.get('lp_client')) {
             // Create our own instance of the LP client.
-            this.set("lp_client", new Y.lp.client.Launchpad());
+            this.set('lp_client', new Y.lp.client.Launchpad());
         }
     },
 
@@ -534,7 +538,7 @@
     _connectScrollers: function() {
         var self = this;
         var rc = Y.lp.code.branchmergeproposal.reviewcomment;
-        Y.all('td[data-previewdiff-id]').each( function(td) {
+        Y.all('td[data-previewdiff-id]').each(function(td) {
             // Comments from superseded MPs should be ignored.
             if (td.getData('from-superseded') === 'True') {
                 return;
@@ -561,7 +565,7 @@
             return;
         }
         rc_scroller = Y.Node.create('<a href="">Return to add comment</a>')
-            .addClass('review-comment-scroller')
+            .addClass('review-comment-scroller');
         this.get('srcNode').append(rc_scroller);
         rc.link_scroller(rc_scroller, '#add-comment-form', function() {
             if (Y.all('.draft').size() > 0) {
@@ -583,7 +587,7 @@
         var container = this.get('srcNode').one('.diff-content');
         var config = {
             on: {
-                success: function (diff_html) {
+                success: function(diff_html) {
                     container.set('innerHTML', diff_html);
                     self.set_status_updated();
                 },
@@ -642,7 +646,7 @@
 
         var pub_drafts_anchor = Y.one('[id="field.review_type"]');
         if (pub_drafts_anchor !== null) {
-            var pub_drafts_container = Y.one('.publish_drafts_container')
+            var pub_drafts_container = Y.one('.publish_drafts_container');
             if (pub_drafts_container === null) {
                 pub_drafts_container = Y.Node.create(
                     '<div class="publish_drafts_container">');