While writing the patch for #1460022: Prefix all jQuery variables with $ I came across some unused variable declarations that should be removed.

Comments

Jelle_S’s picture

Jelle_S
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: Jelle_S commented
Issue tags: +JavaScript clean-up

Adding tag

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Title: Javascript: Remove unused variable declarations » Remove unused variable declarations

Already in the JS component :) I added this one to the clean-up meta issue.

Jelle_S’s picture

Jelle_S
Primary language Dutch
Location Antwerp, Belgium
CreditAttribution: Jelle_S commented
Title: Remove unused variable declarations » Javascript: Remove unused variable declarations

I was thinking... since we are jshinting everything anyway. Can't we just set the "unused" property in the configuration? That would take care of everything...

(Might be a 'problem' for all the jshinted files that were already committed?)

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Title: Javascript: Remove unused variable declarations » JSHint: Remove unused variable declarations
Status: Active » Postponed

oh yeah, haven't noticed this config option. Looks like a new option actually, my jshint don't recognize it. Sure, update the jshint issue with the details and we'll get to this one once the rest of the files are taken care of.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Postponed » Closed (won't fix)

Actually, i'm changing my point of view on this.

Having useless parameter can help contrib when they copy/paste (let's be honest happens all the time). So it's like documentation. And I'm pretty sure whatever we use to minify will get rid of them.

Also it'd mean a lot of changes to JS files. Already hard enough to get anything done in JS :)

Feel free to reopen if you want to discuss some more.