launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/comment-cleanup into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/comment-cleanup/+merge/113116

Summary
=======
There are some errors in the handling of comments in javascript. To really
deal with those, we need to clean up the js as we have elsewhere.

This branch is the first of a series--it simply cleans up the comment code by
using Y.Base, ensuring we're handling everything in an up to date fashion.

Preimp
======
Began discussions of this arc of work with Curtis Hovey.

Implementation
==============
Replace the piecemeal, though correct, creation of the comment object with a
new, cleaner creation with Y.Base. Do the same with the branch merge proposal
comments.

The latter can probably be moved into their own file, but one thing at a time.

Tests
=====
bin/test -vvct comment --layer=YUI

QA
==
Make sure comments in bugs and merge proposals (and questions?) are working
normally.

LoC
===
This is part of disclosure.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/comment.js
-- 
https://code.launchpad.net/~jcsackett/launchpad/comment-cleanup/+merge/113116
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/comment-cleanup into lp:launchpad.

=== modified file 'lib/lp/app/javascript/comment.js'
--- lib/lp/app/javascript/comment.js	2012-01-17 07:12:24 +0000
+++ lib/lp/app/javascript/comment.js	2012-07-02 21:04:26 +0000
@@ -2,17 +2,7 @@
 
 var namespace = Y.namespace('lp.app.comment');
 
-var Comment = function () {
-        Comment.superclass.constructor.apply(this, arguments);
-};
-
-
-Comment.NAME = 'comment';
-
-Comment.ATTRS = {
-};
-Y.extend(Comment, Y.Widget, {
-
+namespace.Comment = Y.Base.create('comment', Y.Widget, [], {
     /**
      * Initialize the Comment
      *
@@ -202,17 +192,13 @@
     syncUI: function(){
         this.submit_button.set('disabled', !this.validate());
     }
+}, {
+    ATTRS: {},
 });
 
-namespace.Comment = Comment;
-
-var CodeReviewComment = function(){
-        CodeReviewComment.superclass.constructor.apply(this, arguments);
-};
-CodeReviewComment.NAME = 'codereviewcomment';
-
-
-Y.extend(CodeReviewComment, Comment, {
+namespace.CodeReviewComment = Y.Base.create(
+    'codereviewcomment', namespace.Comment, [],
+{
     /**
      * Initialize the CodeReviewComment
      *
@@ -256,7 +242,7 @@
         if (this.get_vote() !== null) {
             return true;
         }
-        return CodeReviewComment.superclass.validate.apply(this);
+        return namespace.Comment.prototype.validate.apply(this);
     },
     /**
      * Make the widget enabled or disabled.
@@ -265,7 +251,7 @@
      * @param disabled A boolean, true if the widget is disabled.
      */
     set_disabled: function(disabled){
-        CodeReviewComment.superclass.set_disabled.call(this, disabled);
+        namespace.Comment.prototype.set_disabled.call(this, disabled);
         this.vote_input.set('disabled', disabled);
         this.review_type.set('disabled', disabled);
     },
@@ -354,7 +340,7 @@
           this.review_type.set('value', '');
           this.vote_input.set('selectedIndex', 0);
           this.in_reply_to = null;
-          CodeReviewComment.superclass.reset_contents.apply(this);
+          namespace.Comment.prototype.reset_contents.apply(this);
     },
     /**
      * Insert the specified HTML into the page.
@@ -370,7 +356,7 @@
         Y.lp.anim.green_flash({node: comment}).run();
     },
     renderUI: function() {
-        CodeReviewComment.superclass.renderUI.apply(this);
+        namespace.Comment.prototype.renderUI.apply(this);
     },
     /**
      * Implementation of Widget.bindUI: Bind events to methods.
@@ -381,7 +367,7 @@
      * @method bindUI
      */
     bindUI: function() {
-        CodeReviewComment.superclass.bindUI.apply(this);
+        namespace.Comment.prototype.bindUI.apply(this);
         this.vote_input.on('keyup', this.syncUI, this);
         this.vote_input.on('change', this.syncUI, this);
         this.review_type.on('keyup', this.syncUI, this);
@@ -397,7 +383,7 @@
      * @method syncUI
      */
     syncUI: function() {
-        CodeReviewComment.superclass.syncUI.apply(this);
+        namespace.Comment.prototype.syncUI.apply(this);
         var review_type_disabled = (this.get_vote() === null);
         this.review_type.set('disabled', review_type_disabled);
     },
@@ -424,8 +410,7 @@
             }
         });
     }
-});
-namespace.CodeReviewComment = CodeReviewComment;
+}, {});
 
-}, "0.1" ,{"requires":["oop", "io", "widget", "node", "lp.client",
+}, "0.1" ,{"requires":["base", "oop", "io", "widget", "node", "lp.client",
                        "lp.client.plugins", "lp.app.errors"]});