Opened 8 years ago

Closed 7 years ago

#15340 closed defect (fixed)

dojox/app lint errors

Reported by: bill Owned by: Ed Chatelain
Priority: undecided Milestone: 1.9
Component: DojoX App Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description

Dojox/app has a number of syntax errors (like trailing commas) plus unused variables. See attached patch (but it's untested) for details.

Attachments (3)

applint.patch​ (9.8 KB) - added by bill 8 years ago.
applint2.patch​ (172.3 KB) - added by bill 7 years ago.
thousands more lint fixes, especially about global variables and removing unused code
jsonErrors.patch​ (863 bytes) - added by bill 7 years ago.
missing comma, missing right curly brace

Download all attachments as: .zip

Change History (14)

Changed 8 years ago by bill

Attachment: applint.patch​ added

comment:1 Changed 7 years ago by bill

Owner: changed from Eric Wang to Ed Chatelain

comment:2 Changed 7 years ago by Ed Chatelain

I am sorry that I did not see this when it was opened two months ago, but I have taken a look at the updates and tested with them. It seems safe, but I am also ok waiting until next release to make these changes. Bill what do you think? Should these changes go into 1.8 or wait?

comment:3 Changed 7 years ago by bill

Hard to justify checking them in now unless they fix an IE error due to a trailing comma. I'd suggest delaying until after the 1.8 release.

comment:4 Changed 7 years ago by Ed Chatelain

#15754 is a duplicate of this ticket.

comment:5 Changed 7 years ago by Ed Chatelain

Milestone: tbd → 1.9
Resolution: → fixed
Status: new → closed

These are all taken care of now.

comment:6 Changed 7 years ago by bill

Resolution: fixed
Status: closed → reopened

No, there are still hundreds of errors, including trailing commas and rampant creation of global variables. I'll attach another patch.

Changed 7 years ago by bill

Attachment: applint2.patch​ added

thousands more lint fixes, especially about global variables and removing unused code

comment:7 Changed 7 years ago by bill

Attached a patch to fix many (but not all) of the problems. The reformatted HTML files are to fix missing closing </div> tags.

comment:8 Changed 7 years ago by bill

I also added TODO's in the patch for errors where the fix wasn't clear.

Also, the commas I added to scene.json make it look right but lint is complaining about extra commas, so I guess there's something else weird about that file.

comment:9 Changed 7 years ago by Ed Chatelain

Resolution: → fixed
Status: reopened → closed

Thanks Bill, these have been taken care of with ​https://github.com/dmachi/dojox_application/pull/191

With the exception of a few where we want to leave the informational unused params in oreder to help developers who can create their own controllers know what will be passed.

It looks like I need to find a better editor, to help me avoid these types of things in the future.

comment:10 Changed 7 years ago by bill

Resolution: fixed
Status: closed → reopened

Thanks, it's looking pretty good now. I just see two apparent syntax errors in two JSON files, which I'll attach another patch for. BTW I didn't test any of my patches; presumably you ran the regression.

Changed 7 years ago by bill

Attachment: jsonErrors.patch​ added

missing comma, missing right curly brace

comment:11 Changed 7 years ago by Ed Chatelain

Resolution: → fixed
Status: reopened → closed

Thanks Bill, I think I have those taken care of now. Thanks for the patches, and yes I have been testing all of the fixes.

Note: See TracTickets for help on using tickets.