Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Switched from JSHint to ESLint #542

Merged
merged 1 commit into from
Aug 25, 2016
Merged

Switched from JSHint to ESLint #542

merged 1 commit into from
Aug 25, 2016

Conversation

ficristo
Copy link
Collaborator

Similar work to the one in Brackets.
I've added a .bracket.json file so I could change the prefered linter. (And remove the inline jslint declarations)
I've added an .eslint.json file under tasks/ because Grunt isn't compatible with the rule no-invalid-this.
There should not be any change in behaviour.

(function () {
"use strict";

var util = require("util"),
server = require("./Server"),
Copy link
Contributor

Choose a reason for hiding this comment

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

why this change?

@ficristo
Copy link
Collaborator Author

@zaggino They are both unused.

@zaggino
Copy link
Contributor

zaggino commented Dec 14, 2015

Yep, but even requiring them can have side-effects, like parts of the module being executed, event listeners attached and so on. Are you sure not requiring them won't break anything? Instead of server = require("./Server") you can do just require("./Server"); to be sure nothing breaks.

@ingorichter
Copy link
Contributor

What is the status of this? Any progress? @ficristo @zaggino.

@zaggino
Copy link
Contributor

zaggino commented Jan 8, 2016

Seems finished to me

@ficristo
Copy link
Collaborator Author

ficristo commented Jan 8, 2016

I've updated grunt-eslint to the latest version, enabled the no-trailing-spaces rule and I've squashed the commits.
Now should be ready.

@nethip
Copy link
Contributor

nethip commented Jun 21, 2016

@ficristo I tried playing with this branch. There is one side effect because of this change. After building brackets-shell with this branch and opening brackets-shell source folder, with the built shell, JS hints don't appear anymore.

screen shot 2016-06-21 at 10 54 39 pm

The below snapshot is with the shell built from the release branch.

screen shot 2016-06-21 at 10 55 24 pm

Also I have observed that if I open any file under node-core with any other project, JSHints kick in.
screen shot 2016-06-21 at 10 56 45 pm

@ficristo
Copy link
Collaborator Author

I cannot replicate the issue on the node-core side.
As of now there is a newer version of ESLint which will rise some more warnings and would like to update to it.
But before can we get a consensus on what to do with these:
adobe/brackets#11984
adobe/brackets#11988
(For not seeing JSLint anymore: we need the ESLint extension in core or I need to reenable JSLint like I did on Brackets side)

@ficristo
Copy link
Collaborator Author

I've reintroduced JSLint so we can merge this.
I removed the IIFE and so deindented the code, that's why you see a lot of changes.
You can look at this link https://github.com/adobe/brackets-shell/pull/542/files?w=1 to see code changes only.

@zaggino
Copy link
Contributor

zaggino commented Aug 25, 2016

Gone through it, LGTM

@zaggino zaggino merged commit cebe349 into adobe:master Aug 25, 2016
@ficristo ficristo deleted the eslint branch August 26, 2016 07:45
@ficristo ficristo added this to the Release 1.8 milestone Aug 26, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants