Bug 135916 - Web Inspector: eslint configuration should be stored as .eslintrc
Summary: Web Inspector: eslint configuration should be stored as .eslintrc
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P3 Normal
Assignee: Jonathan Wells
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-08-13 16:52 PDT by Jonathan Wells
Modified: 2014-08-13 18:13 PDT (History)
5 users (show)

See Also:


Attachments
[PATCH] adding .eslintrc (2.80 KB, patch)
2014-08-13 16:57 PDT, Jonathan Wells
joepeck: review+
Details | Formatted Diff | Diff
[PATCH] adding .eslintrc, review feedback. (5.10 KB, patch)
2014-08-13 17:33 PDT, Jonathan Wells
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Wells 2014-08-13 16:52:25 PDT
An .eslintrc file will allow eslint, run as standalone or in an editor, to automatically pick up style rules agreed upon for the WebKit project. This will define all the allowed globals (WebInspector for instance) and syntax rules for proper inspector JavaScript style. 

We should start with some basics and grow from there.

For more information:
ESLint configuration overview: http://eslint.org/docs/configuring/
ESLint full list of available rules: http://eslint.org/docs/rules/
Comment 1 Radar WebKit Bug Importer 2014-08-13 16:52:33 PDT
<rdar://problem/18012970>
Comment 2 Jonathan Wells 2014-08-13 16:57:22 PDT
Created attachment 236565 [details]
[PATCH] adding .eslintrc
Comment 3 Joseph Pecoraro 2014-08-13 17:15:01 PDT
Comment on attachment 236565 [details]
[PATCH] adding .eslintrc

View in context: https://bugs.webkit.org/attachment.cgi?id=236565&action=review

I think this is a reasonable starting point. This will be fun to extend!

> Source/WebInspectorUI/.eslintrc:38
> +    "globals": {
> +        "ConsoleAgent": true,
> +        "DOMAgent": true,
> +        "InspectorBackend": true,
> +        "InspectorFrontendHost": true,
> +        "PageAgent": true,
> +        "WebInspector": true
> +    },

All of the agents will need to be defined in here:

    shell> cd Source
    shell> find . | egrep 'protocol\/.*?json$' | xargs basename | sed -e 's/\.json/Agent/'
    ConsoleAgent
    DebuggerAgent
    GenericTypesAgent # Note we this is a supplemental file so you don't want it.
    InspectorDomainAgent # Note this would just be "InspectorAgent".
    ProfilerAgent
    RuntimeAgent
    ApplicationCacheAgent
    CSSAgent
    DatabaseAgent
    DOMAgent
    DOMDebuggerAgent
    DOMStorageAgent
    IndexedDBAgent
    LayerTreeAgent
    NetworkAgent
    PageAgent
    ReplayAgent
    TimelineAgent
    WorkerAgent

And we will want:

    CodeMirror

> Source/WebInspectorUI/.eslintrc:40
> +        "eqeqeq": 0,

Whoa whoa. We certainly want this on! Maybe not inside UserInterface/External for CodeMirror / Esprima.
Comment 4 Jonathan Wells 2014-08-13 17:24:30 PDT
Comment on attachment 236565 [details]
[PATCH] adding .eslintrc

View in context: https://bugs.webkit.org/attachment.cgi?id=236565&action=review

>> Source/WebInspectorUI/.eslintrc:40
>> +        "eqeqeq": 0,
> 
> Whoa whoa. We certainly want this on! Maybe not inside UserInterface/External for CodeMirror / Esprima.

I completely agree, but there were instances of == being used I've found. I'll consider those errors and turn this into a 2, and I'll add an .eslintrc file for CodeMirror and Esprima.
Comment 5 Jonathan Wells 2014-08-13 17:33:19 PDT
Created attachment 236569 [details]
[PATCH] adding .eslintrc, review feedback.
Comment 6 WebKit Commit Bot 2014-08-13 18:13:29 PDT
Comment on attachment 236569 [details]
[PATCH] adding .eslintrc, review feedback.

Clearing flags on attachment: 236569

Committed r172547: <http://trac.webkit.org/changeset/172547>
Comment 7 WebKit Commit Bot 2014-08-13 18:13:32 PDT
All reviewed patches have been landed.  Closing bug.