brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] Eslint #10127

Open core-ai-bot opened 2 years ago

core-ai-bot commented 2 years ago

Issue by ficristo Saturday Sep 12, 2015 at 18:36 GMT Originally opened as https://github.com/adobe/brackets/pull/11693


I added support for ESLint in grunt and removed JSHint. I need to remove all the JSLint comments from the source files. While I tried to preserve the old behavior (actually I want some more rules), I need to fix some rules (the one with 0 in the eslintrc file). And to decide what to do with the default linter in Brackets. Personally I am in favor to remove it completely, people can use the extensions instead. What you all think?


ficristo included the following code: https://github.com/adobe/brackets/pull/11693/commits

core-ai-bot commented 2 years ago

Comment by abose Sunday Sep 13, 2015 at 13:40 GMT


Why do you prefer eslint over jshint for use with brackets?

core-ai-bot commented 2 years ago

Comment by petetnt Sunday Sep 13, 2015 at 14:13 GMT


@abose see this discussion about replacing JSLint in #11644

core-ai-bot commented 2 years ago

Comment by abose Sunday Sep 13, 2015 at 14:47 GMT


got it! @ficristo did you do a grunt build to see if there are any build failures due to eslint problems? There would be jslint annotations listed all over the code base. Is it compatible with eslint annotations?

core-ai-bot commented 2 years ago

Comment by ficristo Sunday Sep 13, 2015 at 15:00 GMT


@abose I added ESLint because I thought was the preferred one by the Brackets community. As pointed out by@petetnt I saw in some issues / forum that was the preferred choice. If you use JSHint and still want stylistic checks you should use JSCS too. I run grunt eslint with the set of rules I written and it passes. As I written in the first post there are some rule disabled. I tried grunt build too and I didn't noticed any problem with JSLint. I didn't try inline comments, but personally I want to remove all of them.

core-ai-bot commented 2 years ago

Comment by IanVS Monday Sep 14, 2015 at 04:29 GMT


@ficristo You may want to consider adding "extends": "eslint:recommended" to your .eslintrc, since as of version 1.0.0, all rules are disabled by default (see http://eslint.org/docs/user-guide/migrating-to-1.0.0). Recommended rules are denoted by "(recommended)" in http://eslint.org/docs/rules/.

core-ai-bot commented 2 years ago

Comment by ficristo Monday Sep 14, 2015 at 07:39 GMT


@IanVS I tried "extends": "eslint:recommended" but there are still too many errors, and I think to prefer to list all the rules.

I would like to land the patch as is and file followup to enable (or remove) the current set of rule. Eventually we also can add other rules.

core-ai-bot commented 2 years ago

Comment by abose Monday Sep 14, 2015 at 07:43 GMT


Moved to milestone 1.6 so that we could work on comprehensive support of es6 related pr's.

core-ai-bot commented 2 years ago

Comment by le717 Wednesday Sep 16, 2015 at 17:32 GMT


Just chiming in here to say a while back I began rewriting the JSHint rules for ESLint (+ incorporating the style guide wiki page). If it helps any, here are set of rules I had already implemented. The Brackets source failed heavily against them (partially from incomplete rule porting) but it was working.

core-ai-bot commented 2 years ago

Comment by zaggino Wednesday Sep 16, 2015 at 20:59 GMT


hi@le717 , how does nodeca/indent differ from eslint's indent rule?

core-ai-bot commented 2 years ago

Comment by petetnt Thursday Sep 17, 2015 at 04:48 GMT


@zaggino ESLint didn't get indendation support until version 0.14 so the plugin was neccessary. It can safely be replaced by native indent now though.

core-ai-bot commented 2 years ago

Comment by le717 Friday Sep 18, 2015 at 14:41 GMT


@zaggino I believe@petetnt is correct. IIRC, ESLint did not have native intent support, so I added that plugin to enforce it (as it is not already being done and manually checking it on code reviews when it could be automatic is not the wisest idea).

core-ai-bot commented 2 years ago

Comment by zaggino Friday Sep 18, 2015 at 20:57 GMT


yes but eslint is now pass 1.0 and has indent support so we should use that one instead of a plugin

core-ai-bot commented 2 years ago

Comment by le717 Saturday Sep 19, 2015 at 14:46 GMT


@zaggino Yes, it would be best to use native indent support. :)

core-ai-bot commented 2 years ago

Comment by ficristo Friday Nov 27, 2015 at 09:39 GMT


Is there a way I can help to move forward this?

core-ai-bot commented 2 years ago

Comment by abose Saturday Nov 28, 2015 at 13:25 GMT


tagging@swmitra

core-ai-bot commented 2 years ago

Comment by zaggino Tuesday Dec 08, 2015 at 03:45 GMT


the PR seems good, is there any reason not to merge this?

core-ai-bot commented 2 years ago

Comment by petetnt Tuesday Dec 08, 2015 at 07:16 GMT


LGTM :+1:

core-ai-bot commented 2 years ago

Comment by abose Tuesday Dec 08, 2015 at 10:12 GMT


Verified build works. Merging.

core-ai-bot commented 2 years ago

Comment by abose Tuesday Dec 08, 2015 at 10:13 GMT


Thanks@ficristo Should ESlint be prefferd for the lint panel in brackets?

core-ai-bot commented 2 years ago

Comment by zaggino Tuesday Dec 08, 2015 at 10:23 GMT


@abose Yes, I believe so

core-ai-bot commented 2 years ago

Comment by petetnt Tuesday Dec 08, 2015 at 10:23 GMT


@abose That would require need integration of the ESLint (similarly how JSLint is integrated now under https://github.com/adobe/brackets/tree/d939740fd5c368c347ac6a8c7cef2941981aa2fb/src/extensions/default IMO

@zaggino has the great ESLint extension so I think he has the most insight on how much work it would require. Personally I would love it :bouquet:

core-ai-bot commented 2 years ago

Comment by zaggino Tuesday Dec 08, 2015 at 10:24 GMT


It's easy to get eslint integrated direclty into Brackets core, I can do a PR for this if you want guys.

core-ai-bot commented 2 years ago

Comment by abose Tuesday Dec 08, 2015 at 10:31 GMT


+1 for ESLint in default extensions in core.

core-ai-bot commented 2 years ago

Comment by ficristo Tuesday Dec 08, 2015 at 10:49 GMT


I agree to switch from JSLint to ESLint as default linter in core.

core-ai-bot commented 2 years ago

Comment by abose Tuesday Dec 08, 2015 at 11:57 GMT


one part of https://github.com/adobe/brackets/issues/11644 will also be addressed by the switch.

core-ai-bot commented 2 years ago

Comment by petetnt Tuesday Dec 08, 2015 at 12:22 GMT


I created #11984 to track the remaining steps of the ESLint integration