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

Clean up some of the vars and loops. #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ljharb
Copy link
Collaborator

@ljharb ljharb commented Oct 25, 2015

There's zero reason in 2015 to use a loop, or "var" - this cleans some of them up.

There's still a bunch left, but I figured I'd tackle just a little bit at a time.

@@ -87,14 +87,9 @@ RegExp.make = (function () {
* @this {!CharRanges}
*/
CharRanges.prototype.toString = function () {
var s = '';
/** @type {!Array.<number>}. */
const ranges = this.ranges;
/** @type {number} */
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to go away since you've gotten rid of n.

@ljharb ljharb force-pushed the patch-1 branch 2 times, most recently from cb6dde9 to 0f0c91b Compare October 25, 2015 15:07
@ljharb
Copy link
Collaborator Author

ljharb commented Oct 25, 2015

I've updated the diff.

In a JS project, cloning and running npm install should be sufficient to install all dependencies required, and npm test should run any required checks and exit nonzero if any fail. That will also allow you to hook travis-ci up to PRs so that you don't have to manually review many of these kinds of things.

There's zero reason in 2015 to use a loop, or "var" - this cleans some of them up.

There's still a bunch left, but I figured I'd tackle just a little bit at a time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants