launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03380
[Merge] lp:~benji/launchpad/lint into lp:launchpad
Benji York has proposed merging lp:~benji/launchpad/lint into lp:launchpad with lp:~gary/launchpad/direct-personal-subscription-actions as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~benji/launchpad/lint/+merge/58370
Fix lots of lint in subscriptions-related JS.
Tests that I ran manually (all of which passed):
lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.html
lib/lp/registry/javascript/tests/test_distroseries.html
lib/lp/registry/javascript/tests/test_milestone_table.html
lib/lp/registry/javascript/tests/test_structural_subscription.html
lib/lp/registry/javascript/tests/test_timeline.html
lib/lp/registry/javascript/tests/timeline-iframe.html
lib/lp/bugs/javascript/tests/test_me_too.html
lib/lp/bugs/javascript/tests/test_subscriber.html
lib/lp/bugs/javascript/tests/test_subscription.html
--
https://code.launchpad.net/~benji/launchpad/lint/+merge/58370
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~benji/launchpad/lint into lp:launchpad.
=== modified file 'lib/lp/app/javascript/ajax_log.js'
--- lib/lp/app/javascript/ajax_log.js 2011-03-31 12:51:30 +0000
+++ lib/lp/app/javascript/ajax_log.js 2011-04-19 18:16:48 +0000
@@ -67,7 +67,7 @@
}).run();
/* Signify a new entry has been added to the log.
*/
- if (ajax_menu_animating == false) {
+ if (ajax_menu_animating === false) {
flash_menu.run();
ajax_menu_animating = true;
}
=== modified file 'lib/lp/app/javascript/calendar.js'
--- lib/lp/app/javascript/calendar.js 2010-07-15 11:06:14 +0000
+++ lib/lp/app/javascript/calendar.js 2011-04-19 18:16:48 +0000
@@ -23,7 +23,7 @@
*/
var pad_with_zero = function(num) {
num_as_string = String(num);
- if (num_as_string.length == 1) {
+ if (num_as_string.length === 1) {
num_as_string = "0" + num_as_string;
}
return num_as_string;
=== modified file 'lib/lp/app/javascript/choice.js'
--- lib/lp/app/javascript/choice.js 2011-03-07 01:44:54 +0000
+++ lib/lp/app/javascript/choice.js 2011-04-19 18:16:48 +0000
@@ -51,7 +51,7 @@
var cb = widget.get('contentBox');
var value = widget.get('value');
Y.Array.each(config.items, function(item) {
- if (item.value == value) {
+ if (item.value === value) {
cb.one('span').addClass(item.css_class);
} else {
cb.one('span').removeClass(item.css_class);
@@ -59,6 +59,6 @@
});
});
widget.render();
-}
+};
}, "0.1", {"requires": ["lazr.choiceedit", "lp.client.plugins"]});
=== modified file 'lib/lp/app/javascript/client.js'
--- lib/lp/app/javascript/client.js 2011-04-13 06:03:33 +0000
+++ lib/lp/app/javascript/client.js 2011-04-19 18:16:48 +0000
@@ -27,10 +27,10 @@
module.log_object = function(o, name) {
var result;
var format = function(value) {
- if (typeof value == 'string') {
+ if (typeof value === 'string') {
value = value.substring(0, 200); // Truncate long strings.
return '"' + value + '"';
- } else if (typeof value == 'function') {
+ } else if (typeof value === 'function') {
// Only log the function parameters instead
// of the whole code block.
return String(value).split(" {")[0];
@@ -45,13 +45,14 @@
var items = [];
var keys = [];
var key;
+ var index;
for (key in collection) {
if (collection.hasOwnProperty(key)) {
keys.push(key);
}
}
keys.sort();
- for (var index in keys) {
+ for (index in keys) {
if (keys.hasOwnProperty(index)) {
key = keys[index];
var value;
@@ -68,7 +69,7 @@
return items.join(',\n ');
};
- if (o === null || typeof o == 'string' || typeof o == 'function') {
+ if (o === null || typeof o === 'string' || typeof o === 'function') {
result = format(o);
} else {
result = '(direct-attributes)\n ' + introspect(o);
@@ -102,7 +103,7 @@
Does nothing to a relative URI that includes the root.*/
var host_start = uri.indexOf('//');
- if (host_start != -1) {
+ if (host_start !== -1) {
var host_end = uri.indexOf('/', host_start+2);
// eg. "http://www.example.com/api/devel/foo";;
// Don't try to insert the service base into what was an
@@ -152,7 +153,7 @@
module.get_field_uri = function(base_uri, field_name) {
base_uri = module.normalize_uri(base_uri);
field_name = escape(field_name);
- if (base_uri.charAt(base_uri.length - 1) == '/') {
+ if (base_uri.charAt(base_uri.length - 1) === '/') {
return base_uri + field_name;
} else {
return base_uri + '/' + field_name;
@@ -181,59 +182,73 @@
function update_cached_object(cache_name, cache, entry)
{
- var fields_changed = new Array();
- for (var name in cache) {
- var old_value = cache[name];
- var new_value = entry.get(name);
- if (name != 'lp_html') {
- if (old_value != new_value) {
- fields_changed.push(name);
- cache[name] = new_value;
- var field_updated_event_name =
- 'lp:' + cache_name + ':' + name + ':changed';
- var new_value_html = entry.getHTML(name);
- var event = {
- 'name': name,
- 'old_value': old_value,
- 'new_value': new_value,
- 'new_value_html': new_value_html,
- 'entry': entry
- };
- Y.fire(field_updated_event_name, event);
+ var fields_changed = [];
+ var name;
+ var html_name;
+ for (name in cache) {
+ if (cache.hasOwnProperty(name)) {
+ var old_value = cache[name];
+ var new_value = entry.get(name);
+ if (name !== 'lp_html') {
+ if (old_value !== new_value) {
+ fields_changed.push(name);
+ cache[name] = new_value;
+ var field_updated_event_name =
+ 'lp:' + cache_name + ':' + name + ':changed';
+ var new_value_html = entry.getHTML(name);
+ var event = {
+ 'name': name,
+ 'old_value': old_value,
+ 'new_value': new_value,
+ 'new_value_html': new_value_html,
+ 'entry': entry
+ };
+ Y.fire(field_updated_event_name, event);
+ }
}
- }
- else {
- // Since we don't care here about the content, we aren't using the
- // values here to determine if the field has changed, so we can just
- // update the cache.
- for (var html_name in old_value) {
- old_value[html_name] = new_value[html_name];
+ else {
+ // Since we don't care here about the content, we aren't using the
+ // values here to determine if the field has changed, so we can just
+ // update the cache.
+ for (html_name in old_value) {
+ if (old_value.hasOwnProperty(html_name)) {
+ old_value[html_name] = new_value[html_name];
+ }
+ }
}
}
}
if (fields_changed.length > 0) {
var event_name = 'lp:' + cache_name + ':changed';
- var event = {
+ var event_ = {
'fields_changed': fields_changed,
'entry': entry
};
- Y.fire(event_name, event);
+ Y.fire(event_name, event_);
}
}
module.update_cache = function(entry) {
- if (!entry) return;
+ if (!entry) {
+ return;
+ }
var original_uri = entry.lp_original_uri;
var full_uri = module.get_absolute_uri(original_uri);
- for (var name in LP.cache) {
- var cached_object = LP.cache[name];
- if (!Y.Lang.isValue(cached_object))
+ var name;
+ var cached_object;
+ for (name in LP.cache) {
+ if (LP.cache.hasOwnProperty(name)) {
+ cached_object = LP.cache[name];
+ /*jslint continue:true*/
+ if (!Y.Lang.isValue(cached_object)) {
continue;
- if (cached_object['self_link'] == full_uri) {
- Y.log(name + ' cached object has been updated.');
- update_cached_object(name, cached_object, entry);
+ }
+ if (cached_object.self_link === full_uri) {
+ Y.log(name + ' cached object has been updated.');
+ update_cached_object(name, cached_object, entry);
+ }
}
}
};
@@ -246,7 +261,7 @@
var representation, wrapped;
if (old_on_success) {
var media_type = response.getResponseHeader('Content-Type');
- if (media_type.substring(0,16) == 'application/json') {
+ if (media_type.substring(0,16) === 'application/json') {
representation = Y.JSON.parse(response.responseText);
display_notifications(
response.getResponseHeader('X-Lazr-Notifications'));
@@ -286,21 +301,22 @@
* @param notifications An json encoded array of (level, message) tuples.
*/
function display_notifications(notifications) {
- if (notifications === undefined)
+ if (notifications === undefined) {
return;
+ }
var notifications_by_level = {
'level10': {
- 'notifications': new Array()
+ 'notifications': []
},
'level20': {
- 'notifications': new Array()
+ 'notifications': []
},
'level30': {
- 'notifications': new Array()
+ 'notifications': []
},
'level40': {
- 'notifications': new Array()
+ 'notifications': []
}
};
@@ -320,7 +336,7 @@
var css_class = NOTIFICATION_INFO[key].css_class;
var node = Y.Node.create("<div class='"+css_class+"'/>");
node.set('innerHTML', notification);
- if (last_message==null) {
+ if (last_message === null) {
var div = Y.one('div#request-notifications');
div.insert(node);
} else {
@@ -395,6 +411,7 @@
/* Initialize a resource with its representation and URI. */
this.lp_client = client;
this.lp_original_uri = uri;
+ var key;
for (key in representation) {
if (representation.hasOwnProperty(key)) {
this[key] = representation[key];
@@ -427,8 +444,8 @@
on.failure = function(ignore, response, args) {
var client = args[0];
var original_url = args[1];
- if (response.status == module.HTTP_NOT_FOUND ||
- response.status == module.HTTP_SEE_ALSO) {
+ if (response.status === module.HTTP_NOT_FOUND ||
+ response.status === module.HTTP_SEE_ALSO) {
var file = new HostedFile(client, original_url);
return on_success(file);
} else if (old_on_failure !== undefined) {
@@ -495,6 +512,7 @@
// Copy the representation keys into our own set of attributes, and add
// an attribute-change event listener for caching purposes.
+ var key;
for (key in representation) {
if (representation.hasOwnProperty(key)) {
this.addAttr(key, {value: representation[key]});
@@ -510,7 +528,7 @@
Entry.prototype.mark_as_dirty = function(event) {
/* Respond to an event triggered by modification to an Entry's field. */
- if (event.newVal != event.prevVal) {
+ if (event.newVal !== event.prevVal) {
this.dirty_attributes.push(event.attrName);
}
};
@@ -596,6 +614,7 @@
/* Retrieve the value of a named GET operation on the given URI. */
var parameters = config.parameters;
var data = module.append_qs("", "ws.op", operation_name);
+ var name;
for (name in parameters) {
if (parameters.hasOwnProperty(name)) {
data = module.append_qs(data, name, parameters[name]);
@@ -609,8 +628,10 @@
/* Perform a named POST operation on the given URI. */
var on = Y.merge(config.on);
var parameters = config.parameters;
+ var data;
+ var name;
uri = module.normalize_uri(uri);
- var data = module.append_qs(data, "ws.op", operation_name);
+ data = module.append_qs(data, "ws.op", operation_name);
for (name in parameters) {
if (parameters.hasOwnProperty(name)) {
data = module.append_qs(data, name, parameters[name]);
@@ -620,7 +641,7 @@
var old_on_success = on.success;
on.success = function(unknown, response, args) {
- if (response.status == module.HTTP_CREATED) {
+ if (response.status === module.HTTP_CREATED) {
// A new object was created as a result of the operation.
// Get that object and run the callback on it instead.
var new_location = response.getResponseHeader("Location");
@@ -656,6 +677,7 @@
"Content-Type": "application/json",
"X-Content-Type-Override": "application/json"
};
+ var name;
if (headers !== undefined) {
for (name in headers) {
if (headers.hasOwnProperty(name)) {
@@ -758,7 +780,7 @@
return function(ioId, o) {
self.clearProgressUI();
// If it was a timeout...
- if (o.status == 503) {
+ if (o.status === 503) {
self.showError(
'Timeout error, please try again in a few minutes.');
// If it was a server error...
@@ -771,7 +793,7 @@
}
self.showError(server_error);
// Otherwise we send some sane text as an error
- } else if (o.status == 412){
+ } else if (o.status === 412){
self.showError(o.status + ' ' + o.statusText);
} else {
self.showError(self.get_generic_error(o));
@@ -802,7 +824,7 @@
return null;
}
return result[1];
-}
+};
FormErrorHandler.prototype.get_generic_error = function(response) {
@@ -812,7 +834,7 @@
else {
return response.status + ' ' + response.statusText;
}
-}
+};
module.FormErrorHandler = FormErrorHandler;
@@ -940,7 +962,7 @@
// Save the config object that the user passed in so that we can pass
// any extra parameters through to the lp.client constructor.
this.extra_config = config || {};
- this.extra_config['accept'] = 'application/json;include=lp_html';
+ this.extra_config.accept = 'application/json;include=lp_html';
// Save a reference to the original _saveData()
//method before wrapping it.
=== modified file 'lib/lp/bugs/javascript/bug_subscription.js'
--- lib/lp/bugs/javascript/bug_subscription.js 2011-03-23 16:28:51 +0000
+++ lib/lp/bugs/javascript/bug_subscription.js 2011-04-19 18:16:48 +0000
@@ -45,7 +45,7 @@
slide_in_anim.run();
Y.each(subscription_radio_buttons, function(subscription_button) {
subscription_button.on('click', function(e) {
- if(e.target.get('value') == 'update-subscription') {
+ if(e.target.get('value') === 'update-subscription') {
slide_out_anim.run();
} else {
slide_in_anim.run();
@@ -55,5 +55,5 @@
}
};
-}, "0.1", {"requires": ["dom", "node", "io-base", "lazr.anim", "lazr.effects",
+}, "0.1", {"requires": ["dom", "node", "io-base", "lazr.anim", "lazr.effects"
]});
=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js 2011-04-12 12:16:03 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js 2011-04-19 18:16:48 +0000
@@ -265,7 +265,7 @@
},
failure: function(id, request) {
dupe_span.removeClass('update-in-progress-message');
- if (request.status == 400) {
+ if (request.status === 400) {
duplicate_form_overlay.showError(
new_dup_id + ' is not a valid bug number or' +
' nickname.');
@@ -384,9 +384,12 @@
display_privacy_notification();
} else {
if (bugs_private_notification_enabled) {
- if (Y.one('.global-notification').hasClass('hidden')) {
- Y.one('.portlet.private').setStyle('color', '#333');
- Y.one('.portlet.private').setStyle('background-color', '#fbfbfb');
+ if (Y.one('.global-notification').hasClass(
+ 'hidden')) {
+ Y.one('.portlet.private').setStyles({
+ color: '#333',
+ backgroundColor: '#fbfbfb'
+ });
}
}
Y.one('body').replaceClass('private', 'public');
@@ -440,9 +443,10 @@
this is because we have no way to use feature flags in
css directly. This should be remove if the feature
is accepted. */
- Y.one('body').addClass('feature-flag-bugs-private-notification-enabled');
+ var body = Y.one('body');
+ body.addClass('feature-flag-bugs-private-notification-enabled');
/* Set the visible flag so that the content moves down. */
- Y.one('body').addClass('global-notification-visible');
+ body.addClass('global-notification-visible');
if (Y.one('.global-notification').hasClass('hidden')) {
Y.one('.global-notification').addClass('transparent');
@@ -722,7 +726,7 @@
status_choice_edit.on('save', function(e) {
var cb = status_choice_edit.get('contentBox');
Y.Array.each(conf.status_widget_items, function(item) {
- if (item.value == status_choice_edit.get('value')) {
+ if (item.value === status_choice_edit.get('value')) {
cb.addClass(item.css_class);
} else {
cb.removeClass(item.css_class);
@@ -755,7 +759,7 @@
importance_choice_edit.on('save', function(e) {
var cb = importance_choice_edit.get('contentBox');
Y.Array.each(conf.importance_widget_items, function(item) {
- if (item.value == importance_choice_edit.get('value')) {
+ if (item.value === importance_choice_edit.get('value')) {
cb.addClass(item.css_class);
} else {
cb.removeClass(item.css_class);
@@ -811,9 +815,9 @@
inline_combo.set('value', null);
Y.Array.each(
milestone_choice_edit.get('items'), function(item) {
- if (item.value == milestone_choice_edit.get('value')) {
+ if (item.value === milestone_choice_edit.get('value')) {
inline_combo.all('option').each(function(opt) {
- if (opt.get('innerHTML') == item.name) {
+ if (opt.get('innerHTML') === item.name) {
opt.set('selected', true);
}
});
@@ -853,7 +857,8 @@
var yesno_content_template =
"<p>{person_name} did not previously have any " +
"assigned bugs in {pillar}.</p>" +
- "<p>Do you really want to assign them to this bug?</p>";
+ "<p>Do you really want to assign them to this bug?"+
+ "</p>";
var yesno_content = Y.Lang.substitute(
yesno_content_template,
{person_name: person, pillar: pillar});
@@ -881,7 +886,7 @@
};
var step_title =
- (conf.assignee_vocabulary == 'ValidAssignee') ?
+ (conf.assignee_vocabulary === 'ValidAssignee') ?
"Search for people or teams" :
"Select a team of which you are a member";
var assignee_picker = Y.lp.app.picker.addPickerPatcher(
@@ -1003,7 +1008,7 @@
var others_affected_count = this.get('others_affected_count');
var source_names = this._getSourceNames(others_affected_count);
Y.each(this.get('items'), function(item) {
- if (item.value in source_names) {
+ if (source_names.hasOwnProperty(item.value)) {
item.source_name = source_names[item.value];
}
});
@@ -1017,7 +1022,7 @@
_getSourceNames: function(others_affected_count) {
var source_names = {};
// What to say when the user is marked as affected.
- if (others_affected_count == 1) {
+ if (others_affected_count === 1) {
source_names[true] = (
'This bug affects you and 1 other person');
}
@@ -1030,7 +1035,7 @@
source_names[true] = 'This bug affects you';
}
// What to say when the user is marked as not affected.
- if (others_affected_count == 1) {
+ if (others_affected_count === 1) {
source_names[false] = (
'This bug affects 1 person, but not you');
}
=== modified file 'lib/lp/bugs/javascript/subscriber.js'
--- lib/lp/bugs/javascript/subscriber.js 2011-02-24 00:23:04 +0000
+++ lib/lp/bugs/javascript/subscriber.js 2011-04-19 18:16:48 +0000
@@ -90,7 +90,7 @@
Y.all('#subscribers-links div').each(function(link) {
var name = link.one('a').getAttribute('name');
- if (name == display_name) {
+ if (name === display_name) {
already_subscribed = true;
}
});
@@ -117,7 +117,7 @@
*/
'is_current_user_subscribing': function() {
return (
- this.get('subscriber').get('name') ==
+ this.get('subscriber').get('name') ===
this.get('person').get('name')
);
},
@@ -174,11 +174,11 @@
if (Y.Lang.isValue(text)) {
var link = this.get('link');
link.set('innerHTML', text);
- if (text == namespace.subscription_labels.SUBSCRIBE) {
+ if (text === namespace.subscription_labels.SUBSCRIBE) {
link.removeClass('modify remove');
link.removeClass('modify edit');
link.addClass('add');
- } else if (text == namespace.subscription_labels.EDIT) {
+ } else if (text === namespace.subscription_labels.EDIT) {
link.removeClass('add');
link.addClass('modify edit');
} else {
=== modified file 'lib/lp/bugs/javascript/tests/test_subscriber.js'
--- lib/lp/bugs/javascript/tests/test_subscriber.js 2011-02-25 03:54:13 +0000
+++ lib/lp/bugs/javascript/tests/test_subscriber.js 2011-04-19 18:16:48 +0000
@@ -39,7 +39,7 @@
'The escaped user uri should be the same as the unescaped uri.');
Y.Assert.isNull(
this.subscriber.get('user_node'),
- 'User node should not be known and should be null at this point.');
+ 'User node should not be known and be null at this point.');
Y.Assert.areSame(
'',
this.subscriber.get('css_name'),
@@ -172,9 +172,9 @@
// LP is global.
window.LP = {
cache: {},
- links: {},
+ links: {}
};
- Y.lp.client.Launchpad = function() {}
+ Y.lp.client.Launchpad = function() {};
},
tearDown: function() {
=== modified file 'lib/lp/bugs/javascript/tests/test_subscription.js'
--- lib/lp/bugs/javascript/tests/test_subscription.js 2011-04-19 18:16:47 +0000
+++ lib/lp/bugs/javascript/tests/test_subscription.js 2011-04-19 18:16:48 +0000
@@ -1255,8 +1255,7 @@
Y.ArrayAssert.itemsAreEqual(
[],
direct_info.increases);
- },
-
+ }
}));
@@ -1476,7 +1475,7 @@
Y.Assert.areEqual(
'direct-subscription', node.get('id'));
Y.Assert.isTrue(node.get('text').indexOf(expected_text) !== -1);
- },
+ }
}));
=== modified file 'lib/lp/code/javascript/branch.subscription.js'
--- lib/lp/code/javascript/branch.subscription.js 2011-02-24 00:23:04 +0000
+++ lib/lp/code/javascript/branch.subscription.js 2011-04-19 18:16:48 +0000
@@ -60,7 +60,8 @@
* @attribute direct_subscribers {Y.Node} Wrapper div for the subscriber list.
* @attribute self_subscribe {Y.Node} Container for "Subscribe" link.
*/
-var SubscriptionWidget = function(config) {
+var SubscriptionWidget;
+SubscriptionWidget = function(config) {
SubscriptionWidget.superclass.constructor.apply(this, arguments);
};
@@ -84,7 +85,8 @@
var form_url = this.get("self_subscribe").get("href") + '/++form++';
var lp_client = this._lp_client;
- var form_overlay = new Y.lazr.FormOverlay({
+ var form_overlay;
+ form_overlay = new Y.lazr.FormOverlay({
headerContent: "<h2>Subscribe to branch</h2>",
form_submit_button: Y.Node.create(
'<button type="submit" name="field.actions.subscribe" ' +
=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-19 01:17:31 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js 2011-04-19 18:16:48 +0000
@@ -627,7 +627,7 @@
Assert.isTrue(submit_button.hasClass('spinner'));
Assert.isFalse(submit_button.hasClass('lazr-pos'));
// Now we resume the call to trigger the failure.
- this.configuration.lp_client.named_post.resume()
+ this.configuration.lp_client.named_post.resume();
// The spinner is gone.
Assert.isTrue(submit_button.hasClass('lazr-pos'));
Assert.isFalse(submit_button.hasClass('spinner'));
