JS Syntax, and fix selector.

[jbeales]

There's a place that I think we're trying to use jQuery to select something with the class wpsc-visitor-meta, but the dot is missing. Also, a trailing comma has been removed, (my IDE is yelling at me).

Also, JSHint is telling me about a bunch of more stylistic or performance issues in wp-e-commerce.js, such as duplicate variable declarations, duplicate jQuery selectors, (making jQuery do the selection twice instead of doing it once and storing the result in a variable), and unneeded variable assignments, like this:

var message = format.replace("%s",label);
return message;

I'm willing to do a cleanup here, is that something that we would like done, or are there reasons, (like clarity), that to ignore the JSHint warnings?

[instinct]

We use jshint on Kiwi.js - it's good. No objections from me. Anybody else?

Sent from my iPhone

On 15/03/2015, at 6:54 am, John Beales [email protected] wrote:

There's a place that I think we're trying to use jQuery to select something with the class wpsc-visitor-meta, but the dot is missing. Also, a trailing comma has been removed, (my IDE is yelling at me).

Also, JSHint is telling me about a bunch of more stylistic or performance issues in wp-e-commerce.js, such as duplicate variable declarations, duplicate jQuery selectors, (making jQuery do the selection twice instead of doing it once and storing the result in a variable), and unneeded variable assignments, like this:

var message = format.replace("%s",label);
return message;
I'm willing to do a cleanup here, is that something that we would like done, or are there reasons, (like clarity), that to ignore the JSHint warnings?

—
Reply to this email directly or view it on GitHub.

[JustinSainton]

@jbeales We've actually been using jshint for about 3.5 months - only on new stuff, and only for committers using that workflow/grunt task. We need a CONTRIBUTING.MD, which would clarify/specify that PRs not using the grunt workflow would not be merged.

That said - yes, pretty much any front-end JS we have has, last I recall, over 100+ JShint failures/warnings. Lots of the admin JS is much better, but some is not.

[jbeales]

I'll fire up Grunt then and see what it tells me, although a quick glance looks like it just runs JSHint and something for translations. I've also got JSHint turned on in PhpStorm, which is where it's doing most of its yelling.

I'll clean up wp-e-commerce.js, it should be pretty quick & easy.

[instinct]

Awesome. Then let's all start preparing a blog post and a minor release.

d

Sent from my iPhone

On 16/03/2015, at 7:30 am, John Beales [email protected] wrote:

I'll fire up Grunt then and see what it tells me, although a quick glance looks like it just runs JSHint and something for translations. I've also got JSHint turned on in PhpStorm, which is where it's doing most of its yelling.

I'll clean up wp-e-commerce.js, it should be pretty quick & easy.

—
Reply to this email directly or view it on GitHub.

[JeffPyeBrook]

@jbeales with the cleanup we also need to look at and integrate #1780 #1786 #1787

[jbeales]

I'll see what I can do. Some of those look like a PITA to resolve, (like a possible race condition setting a cookie).

[JeffPyeBrook]

@jbeales look at #1802 before you think about the cookie changes, it changes the process to be comaptible with current browser js implementations , and based on testing very much more reliable

[JeffPyeBrook]

... and also has some diagnostics built in to help the support folks when they run into odd server configurations that might not be handling cookies the way we need them