Fix linting issues and update some of the rules #36

molant · 2017-03-12T21:51:22Z

Added the TypeScript plugin,
Updated some of the eslint rules to be more compatible with TypeScript
Refactored the code to require less import for types and thus avoiding triggering the no-unused-variables
Added more spaces to the functions to make @alrra happy

This one supersedes pull #18

…TODOs pending)

…g and passing options to it. Added request module to jsdom collector (using the headers options). Clean up some of the todos and homogeneized code

alrra · 2017-03-12T22:58:23Z

.eslintrc.json

@@ -10,7 +10,8 @@
    },
    "parser": "typescript-eslint-parser",
    "plugins": [
-        "import"
+        "import",
+        "typescript"


@molant You also need to add eslint-plugin-typescript to the package.json file.

Added, good catch 🐟

alrra · 2017-03-12T23:06:07Z

.eslintrc.json

-        "yoda": ["error", "never"]
+        "yoda": ["error", "never"],
+
+        "typescript/type-annotation-spacing": "error"


Maybe also add: typescript/no-unused-vars?

I don't see that rule in the supported ones. Where is it coming from?

It seems it's not yet in any published npm version.

alrra · 2017-03-12T23:10:25Z

src/lib/sonar.ts

@@ -8,13 +8,12 @@
 // ------------------------------------------------------------------------------

 import { EventEmitter2 as EventEmitter } from 'eventemitter2';
-import * as _ from 'lodash';
+// import * as _ from 'lodash';


Why not just remove it until we actually use it?

Completely forgot about this one

alrra · 2017-03-12T23:12:18Z

src/bin/sonar.ts

@@ -15,7 +15,9 @@ const debug = (process.argv.includes('--debug'));

 // must do this initialization *before* other requires in order to work
 if (debug) {
+


IMHO, for the cases where there is just one line, it's overkill.

But we need to enable it in the entrance point for other to work as the comment says. Are you referring to something else?

Are you referring to something else?

Sorry for the confusion. I was referring to the empty lines. When there is just a single line (i.e.: require('debug').enable('sonar:*,-sonar:code-path');), it's overkill to add empty lines around it.

Part of the rule. Don't think we can configure it at that level.

I've removed that rule and increase the number of max empty lines to 2 so the code can breath a bit more for you.

alrra · 2017-03-12T23:15:31Z

src/lib/collectors/jsdom.ts

@@ -46,45 +45,56 @@ const defaultOptions = {
 };

 const builder: CollectorBuilder = (server: Sonar, config): Collector => {
+


When there are groups, like here, it makes sense to have the empty line. ✨

alrra · 2017-03-12T23:24:28Z

src/lib/types.ts

- * @fileoverview Interfaces and enums used in different files in Sonar that do not belong in an specific place.
- * @author Anton Molleda (@molant)
- */
+'use strict';


Maybe use --alwaysStrict?

AFAIK this file shouldn't even be in the final compile. It is used only to check that the types are OK. I added the alwaysStrict to the compiler options and set to never the strict rule in eslint.

molant · 2017-03-13T17:57:21Z

@alrra you can take another look and merge if it is OK.

Change made so that the process of opening the `LICENSE` file is easier for non-technical people.

Also, changed folder structure and add dependencies.

Close #13

Ref: https://docs.npmjs.com/files/package.json#bugs https://docs.npmjs.com/files/package.json#repository

From https://docs.npmjs.com/files/package.json#private: " If you set `"private": true` in your `package.json`, then `npm` will refuse to publish it. This is a way to prevent accidental publication of private repositories. "

* Remove reference to `.travis.yml` as that file is not included. * Add link to the site instead of explaining every line, and add comments only for when the information cannot be found on the site. * Add `trim_trailing_whitespace` property.

Ref https://github.com/blog/2111-issue-and-pull-request-templates

Most people will expect `.sonarrc` to work as in general `.<name>rc` files seem to be the preferred way for people to store CLI specific configurations.

Ref: eslint/eslint#4406 (comment)

For the time being we decided to keep things simple and only support JSON, as supporting multiple formats can complicate things and even create some problems. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Ref: eslint/eslint#4406 (comment)

* Added the TypeScript plugin to ESLint * Tweaked the rules to be more compatible with our style and TS * Refactored the code to require less `imports` so less `no-unused-vars` errors * Fixed all other linting issues

* Add ESLint TypeScript plugin. * Tweak rules to be more compatible with the current style and TypeScript. * Refactor code to require less `imports`, thus, generate less `no-unused-vars` errors. * Fix other linting issues. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - Close #36

molant added 5 commits February 22, 2017 19:58

First scenario with simple double slash rule working (and a bunch of …

85faa30

…TODOs pending)

added more detailed eslint configuration and fixed errors

0c21a41

Added formatters and selection via config. Using collector from confi…

0af0e19

…g and passing options to it. Added request module to jsdom collector (using the headers options). Clean up some of the todos and homogeneized code

Refactor resource loading, minor fixes, initial rule validation

0584e48

Adding support for default configuration

508a9da

molant mentioned this pull request Mar 12, 2017

Fix linting problems #18

Closed

molant requested a review from alrra March 12, 2017 21:53

molant assigned alrra Mar 12, 2017

alrra suggested changes Mar 12, 2017

View reviewed changes

molant changed the title ~~Fixing linting issues and updating some of the rules~~ Fix linting issues and update some of the rules Mar 13, 2017

alrra and others added 14 commits March 13, 2017 12:49

Rename LICENSE to LICENSE.txt

9ca52c8

Change made so that the process of opening the `LICENSE` file is easier for non-technical people.

Updated configuration for TS and add build task

ef535b9

Also, changed folder structure and add dependencies.

Continue migration to TypeScript

e3da3e1

Finish migration to TypeScript

cb3d5d4

Close #13

Use shortcut syntax for package.json fields

c36d8ac

Ref: https://docs.npmjs.com/files/package.json#bugs https://docs.npmjs.com/files/package.json#repository

Mark package as private until it's published

3b167ea

From https://docs.npmjs.com/files/package.json#private: " If you set `"private": true` in your `package.json`, then `npm` will refuse to publish it. This is a way to prevent accidental publication of private repositories. "

Move CONTRIBUTING.md into .github

4337bf3

Ref https://github.com/blog/2111-issue-and-pull-request-templates

Add .sonarrc to the list of config files

3320da3

Most people will expect `.sonarrc` to work as in general `.<name>rc` files seem to be the preferred way for people to store CLI specific configurations.

For consistency use .eslintrc.json

075e1f6

Ref: eslint/eslint#4406 (comment)

Fix minor linting problems

527311b

Remove unneeded if check from src/lib/config.ts

c4bd286

Finish migration to TypeScript for now:

d4574c7

* Added the TypeScript plugin to ESLint * Tweaked the rules to be more compatible with our style and TS * Refactored the code to require less `imports` so less `no-unused-vars` errors * Fixed all other linting issues

alrra closed this in 2afd1a9 Mar 13, 2017

molant deleted the typescript branch March 17, 2017 16:36

molant mentioned this pull request Jan 8, 2019

[Hint] Performance metrics #1634

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix linting issues and update some of the rules #36

Fix linting issues and update some of the rules #36

molant commented Mar 12, 2017

alrra Mar 12, 2017

molant Mar 13, 2017

alrra Mar 12, 2017

molant Mar 13, 2017

alrra Mar 13, 2017

alrra Mar 12, 2017

molant Mar 13, 2017

alrra Mar 12, 2017 •

edited

Loading

molant Mar 13, 2017

alrra Mar 13, 2017

molant Mar 13, 2017

molant Mar 13, 2017

alrra Mar 12, 2017

alrra Mar 12, 2017

molant Mar 13, 2017

molant commented Mar 13, 2017

		@@ -15,7 +15,9 @@ const debug = (process.argv.includes('--debug'));

		// must do this initialization before other requires in order to work
		if (debug) {

		@@ -46,45 +45,56 @@ const defaultOptions = {
		};

		const builder: CollectorBuilder = (server: Sonar, config): Collector => {

Fix linting issues and update some of the rules #36

Fix linting issues and update some of the rules #36

Conversation

molant commented Mar 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alrra Mar 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

molant commented Mar 13, 2017

alrra Mar 12, 2017 •

edited

Loading