Fixes #8714: turn on "unused": true in JSHint #788

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
Member

mikesherov commented Oct 23, 2012

Lots of files changed, but all of the changes are minor and benign. All tests still pass. Please let me know if I need to make any changes. Thanks!

build/release/release.js
@@ -425,7 +425,7 @@ function abort( msg ) {
function walk( methods ) {
var method = methods.shift();
- function next( error ) {
+ function next( ) {
@scottgonzalez

scottgonzalez Oct 23, 2012

Owner

This left some whitespace.

@mikesherov

mikesherov Oct 23, 2012

Member

K. I'll fix all the whitespace in the AM. Just leave the comments.

tests/unit/draggable/draggable_core.js
@@ -5,7 +5,7 @@
var el, offsetBefore, offsetAfter, dragged;
function drag(handle, dx, dy) {
- var element = el.data("draggable").element;
+ el.data("draggable").element;
@scottgonzalez

scottgonzalez Oct 23, 2012

Owner

This doesn't look like it's doing anything now.

@@ -88,6 +88,11 @@ $.widget("ui.dialog", {
options = this.options,
title = options.title || " ",
+ uiDialog,
@scottgonzalez

scottgonzalez Oct 23, 2012

Owner

Is this our new coding standard?

@mikesherov

mikesherov Oct 23, 2012

Member

No, but assigning to unused variables is confusing, and so in this case, because you were assigning in the var statement, you were creating throwaway variables. I just moved the var declarations up, and then only assigned when necessary.

@scottgonzalez

scottgonzalez Oct 23, 2012

Owner

Sorry, I was asking if undefined vars on a separate line at the end instead of on one line at the top was our new coding standards. I've seen people doing different things and I just haven't asked anyone yet.

@mikesherov

mikesherov Oct 23, 2012

Member

Oh, right, that's true. I can do that in a subsequent pull request. I want this to land first so it's not so monolithic.

@scottgonzalez

scottgonzalez Oct 23, 2012

Owner

That's fine. We're doing a big cleanup of dialog right now anyway, so this will definitely get fixed.

@@ -1114,7 +1111,7 @@ function standardSpeed( speed ) {
}
$.fn.extend({
- effect: function( effect, options, speed, callback ) {
@gnarf

gnarf Oct 23, 2012

Owner

Please keep these args around (even if commented)

@gnarf gnarf closed this in e123099 Oct 23, 2012

kborchers added a commit that referenced this pull request Nov 1, 2012

Build: Enable "unused" option in jshint - Remove unused variables fro…
…m codebase. - Closes gh-788

Squashed commit of the following:

commit 7f19f92
Author: Mike Sherov <mike.sherov@gmail.com>
Date:   Tue Oct 23 10:34:28 2012 -0400

    put back in fake args for signatures that we want to keep

commit 257505a
Author: Mike Sherov <mike.sherov@gmail.com>
Date:   Tue Oct 23 08:10:20 2012 -0400

    changes per @scott_gonzalez

commit 1272548
Author: Mike Sherov <mike.sherov@gmail.com>
Date:   Mon Oct 22 22:54:05 2012 -0400

    clean up unused vars in ui directory

commit 563595e
Author: Mike Sherov <mike.sherov@gmail.com>
Date:   Mon Oct 22 22:37:42 2012 -0400

    clean up unused vars in grunt and tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment