Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added eslint based on airbnb config #59

Merged
merged 1 commit into from Mar 31, 2016
Merged

Conversation

zerkms
Copy link
Contributor

@zerkms zerkms commented Mar 31, 2016

I understand that linting is a personal thing but I think that even with the base airbnb config and modified indentation it catches a lot of style inconsistencies.

And it is definitely better than not to have any linter at all.

What do you think?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.214% when pulling 577941f on zerkms:ESLINT_AIRBNB into c1357b8 on tomkp:master.

@tomkp
Copy link
Owner

tomkp commented Mar 31, 2016

I completely agree - I've had this on my todo list for a while.

@zerkms
Copy link
Contributor Author

zerkms commented Mar 31, 2016

So what do you think about adding this then? Majority of code already complies with it, also catches few inconsistencies here and there and probably couple of rules might be changed.

Here is the config I'm using in all my projects:

{
    "extends": "airbnb",

    "parser": "babel-eslint",

    "rules": {
        "max-len": [2, 120, 4, {
            "ignoreUrls": true,
            "ignoreComments": false
        }],
        "indent": [2, 4],
        "react/jsx-indent-props": [2, 4],
        "space-before-function-paren": [2, "never"]
    }
}

@tomkp
Copy link
Owner

tomkp commented Mar 31, 2016

I'm going to merge this in - I guess we need to ensure the codebase is compliant, and then set it up to run automatically (probably as part of the compile step)...

@tomkp tomkp merged commit 016bdc4 into tomkp:master Mar 31, 2016
@tomkp
Copy link
Owner

tomkp commented Mar 31, 2016

Thanks!

@zerkms zerkms deleted the ESLINT_AIRBNB branch March 31, 2016 19:28
@zerkms
Copy link
Contributor Author

zerkms commented Mar 31, 2016

I guess we need to ensure the codebase is compliant

Yep, I will send the PR with changes to the code some time today

and then set it up to run automatically (probably as part of the compile step)

That's also personal: I don't personally do that in any of my projects, since even though I think the style is important, it's okay if some commit does not comply with it. But I'm open for the dialogue :-)

@tomkp
Copy link
Owner

tomkp commented Mar 31, 2016

Great - thanks a lot...
I guess we can make a decision on wether to enforce the linting later... although I think I like the idea of it being mandatory.

@zerkms
Copy link
Contributor Author

zerkms commented Mar 31, 2016

Yep, makes sense

I have fixed style inconsistencies for Pane and Resizer zerkms@132398f

but SplitPane requires some extra effort due to its size so I will do that later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants