Upgrade: eslint & eslint-config-eslint #387

aladdin-add · 2018-07-06T11:22:31Z

the pr upgrade these devdeps:

"eslint": "^5.6.1",
"eslint-config-eslint": "^5.0.0",
"eslint-plugin-node": "^7.0.1",
"istanbul": "^0.4.5",

platinumazure

Is no-unused-vars enabled? If not, should we enable it in this codebase?

aladdin-add · 2018-07-07T03:22:39Z

that's what I am thinking, will do.

aladdin-add · 2018-07-07T15:15:51Z

it would be fine to add eslint-plugin-node. I added it in eslint-config-eslint, but it has not been released yet. Another choice is moving espree to eslint repo (monorepo). I created an issue (eslint/eslint#10575).

platinumazure

Why the change from Chai Assert to Node's assert library? Chai Assert usually gives much more expression assertion messages and works nicely with Mocha.

mysticatea · 2018-07-10T03:15:03Z

I feel built-in assert is nice enough :D

kaicataldo · 2018-10-15T04:46:30Z

Do we want to wait on #391 to land before doing this work? Looks like it might caused some merge conflicts.

aladdin-add · 2018-10-18T14:29:50Z

sure, I like to see istanbul => nyc (it has been done in #391 )

kaicataldo · 2018-10-24T04:05:57Z

#391 has been merged!

aladdin-add · 2018-10-24T15:30:34Z

@kaicataldo I just rebased the newest changes, and thanks for the reminder! :)

kaicataldo · 2018-10-27T19:09:50Z

Makefile.js

- * @returns {Function} The function to pass into a filter method.
- * @private
- */
-function fileType(extension) {


Was this unused?

it was used before the change, but I replaced it with lib/*.js.

Ah, I see - missed that. Thanks 👍

kaicataldo · 2018-10-27T19:11:05Z

Makefile.js

@@ -112,21 +94,21 @@ target.browserify = function() {
    // 1. create temp and build directory
    if (!test("-d", TEMP_DIR)) {
        mkdir(TEMP_DIR);
-        mkdir(TEMP_DIR + "/lib");
+        mkdir(`${TEMP_DIR}/lib`);


Looks like the / is unnecessary here. I wonder if we should just use path.join()?

kaicataldo · 2018-10-27T19:11:09Z

Makefile.js

    }

    if (!test("-d", BUILD_DIR)) {
        mkdir(BUILD_DIR);
    }

    // 2. copy files into temp directory
-    cp("-r", "lib/*", TEMP_DIR + "/lib");
+    cp("-r", "lib/*", `${TEMP_DIR}/lib`);


Looks like the / is unnecessary here. I wonder if we should just use path.join()?

kaicataldo · 2018-10-27T19:11:17Z

Makefile.js

    cp("espree.js", TEMP_DIR);
    cp("package.json", TEMP_DIR);


    // 3. browserify the temp directory
-    nodeCLI.exec("browserify", TEMP_DIR + "espree.js", "-o", BUILD_DIR + "espree.js", "-s espree");
+    nodeCLI.exec("browserify", `${TEMP_DIR}espree.js`, "-o", `${BUILD_DIR}espree.js`, "-s espree");


I wonder if we should just use path.join()?

kaicataldo

LGTM, with a few very minor (non-blocking) suggestions.

I didn't realize we were also using assert in the main ESLint repo. Just so I'm clear, are we trying to move towards using Node's assert module?

Signed-off-by: weiran.zsd <weiran.zsd@alibaba-inc.com>

aladdin-add · 2018-11-04T15:18:06Z

are we trying to move towards using Node's assert module?

no, I don't think we have made the decision.

PR eslint#389 deleted "lib/visitor-keys.js" and replaced all usages with the eslint-visitor-keys package. PR eslint#387 added lib/visitor-keys.js back again for no apparent reason. It looks like eslint#387 has been started before eslint#389, so it needed to integrate those changes and by mistake the file has been added again.

PR #389 deleted "lib/visitor-keys.js" and replaced all usages with the eslint-visitor-keys package. PR #387 added lib/visitor-keys.js back again for no apparent reason. It looks like #387 has been started before #389, so it needed to integrate those changes and by mistake the file has been added again.

aladdin-add added the chore label Jul 6, 2018

platinumazure reviewed Jul 6, 2018

View reviewed changes

aladdin-add changed the title ~~Chore: rm some unused vars~~ Upgrade: eslint & eslint-config-eslint Jul 9, 2018

platinumazure reviewed Jul 9, 2018

View reviewed changes

aladdin-add force-pushed the chore/fix-alert branch 3 times, most recently from 02a9a34 to 8f027ae Compare September 30, 2018 06:49

aladdin-add mentioned this pull request Oct 10, 2018

Upgrade: acorn 6, acorn-jsx 5, and istanbul #391

Merged

aladdin-add force-pushed the chore/fix-alert branch 2 times, most recently from 1a7fbac to b428439 Compare October 18, 2018 04:47

aladdin-add and others added 11 commits October 24, 2018 22:54

Chore: rm some unused vars

c3ed64c

Chore: fix linting errors

9f729ef

Chore: fix linting errors

b0660e6

Chore: fix linting errors in espree.js

25fab56

Upgrade: eslint & eslint-config-eslint

6921baf

Fix: failint tests

3d279b0

Chore: lint tools

2c8d8c5

Update package.json

5192964

Chore: upgrade eslint & clean up

4123009

Chore: fix failing tests(assert.throws)

a8ccc84

Upgrade: eslint & eslint-config-eslint

e6cc4a0

aladdin-add force-pushed the chore/fix-alert branch from b428439 to d9847d6 Compare October 24, 2018 15:19

chore: fix linting errors

d097f4a

aladdin-add force-pushed the chore/fix-alert branch from d9847d6 to d097f4a Compare October 24, 2018 15:23

kaicataldo reviewed Oct 27, 2018

View reviewed changes

kaicataldo approved these changes Oct 27, 2018

View reviewed changes

Chore: use path.join

e4cfca1

Signed-off-by: weiran.zsd <weiran.zsd@alibaba-inc.com>

aladdin-add merged commit 6ebc219 into eslint:master Nov 4, 2018

aladdin-add deleted the chore/fix-alert branch November 4, 2018 15:18

matz3 mentioned this pull request Jul 9, 2021

Chore: Remove obsolete lib/visitor-keys.js #509

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade: eslint & eslint-config-eslint #387

Upgrade: eslint & eslint-config-eslint #387

aladdin-add commented Jul 6, 2018 •

edited

Loading

platinumazure left a comment

aladdin-add commented Jul 7, 2018

aladdin-add commented Jul 7, 2018

platinumazure left a comment

mysticatea commented Jul 10, 2018

kaicataldo commented Oct 15, 2018

aladdin-add commented Oct 18, 2018

kaicataldo commented Oct 24, 2018

aladdin-add commented Oct 24, 2018

kaicataldo Oct 27, 2018

aladdin-add Oct 28, 2018

kaicataldo Oct 28, 2018

kaicataldo Oct 27, 2018

kaicataldo Oct 27, 2018

kaicataldo Oct 27, 2018

kaicataldo left a comment •

edited

Loading

aladdin-add commented Nov 4, 2018

Upgrade: eslint & eslint-config-eslint #387

Upgrade: eslint & eslint-config-eslint #387

Conversation

aladdin-add commented Jul 6, 2018 • edited Loading

platinumazure left a comment

Choose a reason for hiding this comment

aladdin-add commented Jul 7, 2018

aladdin-add commented Jul 7, 2018

platinumazure left a comment

Choose a reason for hiding this comment

mysticatea commented Jul 10, 2018

kaicataldo commented Oct 15, 2018

aladdin-add commented Oct 18, 2018

kaicataldo commented Oct 24, 2018

aladdin-add commented Oct 24, 2018

kaicataldo Oct 27, 2018

Choose a reason for hiding this comment

aladdin-add Oct 28, 2018

Choose a reason for hiding this comment

kaicataldo Oct 28, 2018

Choose a reason for hiding this comment

kaicataldo Oct 27, 2018

Choose a reason for hiding this comment

kaicataldo Oct 27, 2018

Choose a reason for hiding this comment

kaicataldo Oct 27, 2018

Choose a reason for hiding this comment

kaicataldo left a comment • edited Loading

Choose a reason for hiding this comment

aladdin-add commented Nov 4, 2018

aladdin-add commented Jul 6, 2018 •

edited

Loading

kaicataldo left a comment •

edited

Loading