Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove lint from auxiliary JS files #7863

Closed
qed777 mannequin opened this issue Jan 7, 2010 · 6 comments
Closed

Remove lint from auxiliary JS files #7863

qed777 mannequin opened this issue Jan 7, 2010 · 6 comments

Comments

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 7, 2010

JSLint on "The Good Parts" setting, applied to sagenb/data/sage/js/*.js but not notebook_lib.js. The latter will have its own ticket.

For now, I've disabled strict mode, since most (all?) current browsers don't yet have it.

Given the present architecture of the notebook JS library --- use lots of global variables, etc. --- I haven't implemented all of JSLint's suggestions. More generally:

"If you're writing javascript code, [JSLint](http://www.jslint.com/) is a really fine piece of software, too. You don't have to follow its recommendations blindly, but understanding what it says about your code can greatly improve your skills." -- http://jsbeautifier.org/

CC: @TimDumol

Component: notebook

Author: Mitesh Patel

Reviewer: Tim Dumol

Merged: sagenb-0.6

Issue created by migration from https://trac.sagemath.org/ticket/7863

@qed777 qed777 mannequin added this to the sage-4.3.1 milestone Jan 7, 2010
@qed777 qed777 mannequin added c: user interface labels Jan 7, 2010
@qed777 qed777 mannequin assigned williamstein Jan 7, 2010
@qed777

This comment has been minimized.

@qed777 qed777 mannequin added the s: needs review label Jan 7, 2010
@qed777
Copy link
Mannequin Author

qed777 mannequin commented Jan 7, 2010

Attachment: trac_7863-declare_vars_aux_js.patch.gz

Declare vars in functions, etc., in aux JS files. Depends on #7786. sagenb repo.

@qed777
Copy link
Mannequin Author

qed777 mannequin commented Jan 8, 2010

Attachment: trac_7863-declare_vars_aux_js_v2.patch.gz

Rebased vs. #7786 v11. Replaces previous.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 17, 2010

comment:2

Looks good to me. Nice style changes.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 19, 2010

Merged: sagenb-0.6

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 19, 2010

Reviewer: Tim Dumol

@TimDumol TimDumol mannequin removed the s: positive review label Jan 19, 2010
@TimDumol TimDumol mannequin closed this as completed Jan 19, 2010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant