Opened 9 years ago
Closed 8 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)
Change History (14)
Changed 9 years ago by
Attachment: | applint.patch added |
---|
comment:1 Changed 9 years ago by
Owner: | changed from Eric Wang to Ed Chatelain |
---|
comment:2 Changed 9 years ago by
comment:3 Changed 9 years ago by
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:5 Changed 8 years ago by
Milestone: | tbd → 1.9 |
---|---|
Resolution: | → fixed |
Status: | new → closed |
These are all taken care of now.
comment:6 Changed 8 years ago by
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 8 years ago by
Attachment: | applint2.patch added |
---|
thousands more lint fixes, especially about global variables and removing unused code
comment:7 Changed 8 years ago by
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 8 years ago by
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 8 years ago by
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 8 years ago by
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.
comment:11 Changed 8 years ago by
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.
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?