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

Inherit shower-core ESLint config #18

Open
pepelsbey opened this issue Oct 3, 2018 · 14 comments
Open

Inherit shower-core ESLint config #18

pepelsbey opened this issue Oct 3, 2018 · 14 comments
Assignees
Labels
feature New feature or request

Comments

@pepelsbey
Copy link

It would be good to have common JS code style throughout the whole organization.

If you don’t mind, I’d suggest using ; and 4 spaces, just like we do in .eslintrc.yml of shower-core, the main code base of the Shower project. It’s basically airbnb-base with some extras. The same, but in a form if .editorconfig and .stylelintrc exists in both themes.

We can always discuss details, but having a common code style is essential for a project like this.

@mrdimidium
Copy link

Maybe, we will move eslint config in new repository and will be use extrend: 'shower-config'?

@pepelsbey
Copy link
Author

You could do that? :O

@shvaikalesh
Copy link

Latest version here.
I am not supper happy with how airbnb config is unnecessary restrictive, perhaps there is a popular config we can use with less overrides. Definitely 👍 on extracting the config though. I think we should start using scoped packages: @shower/eslint-config, @shower/core, or consider making it pepelsbey-eslint-config.

We can also evaluate prettier: it allows 4 spaces and bracket spacing.

@pepelsbey
Copy link
Author

Yes, I’m all for @shower scoping, but it takes some time to update all packages and I just haven’t done anything like this before :(

@mrdimidium
Copy link

If we want to create @shower scope, maybe will be conveniently using one eslint config in main repository?

@mrdimidium
Copy link

I can create shower-config, but do we need this if we go to scope?

@mrdimidium mrdimidium reopened this Oct 3, 2018
@shvaikalesh
Copy link

Resolution of offline conversation with @pepelsbey: let's hold off with extracting the config and give prettier a try (right after we ship @shower/core@3). We will still need ESLint, but with far less rules and, hopefully, overrides.

@mrdimidium
Copy link

mrdimidium commented Oct 24, 2018

It's a great idea to choose prettier for code style, but I wouldn't want to give up eslint – it can do much more, like look for potential bugs or vulnerabilities. I would suggest setting up a common prettier+eslint bundle and using these tools together

@avdeev
Copy link
Member

avdeev commented Oct 24, 2018

I seems to me that the eslint --fix works like a prettier.

@mrdimidium
Copy link

Not sure, but eslint is much more powerful. Prettier is designed only for formatting, but it does not replace a full-fledged Linter.

@mrdimidium
Copy link

Hey. I tried the eslint+prettier bundle that the kernel uses. Sometimes she does strange things:

Such code:

const semver = require('semver');

const pkg = require('../package.json');

if (!semver.satisfies(process.version, pkg.engines.node)) {
    /* ... */
}

will turn into such:

const semver = require('semver');

if (!semver.satisfies(process.version, pkg.engines.node)) {
    /* ... */
}

const pkg = require('../package.json');

In automatic mode, there is no guarantee that the code will not be damaged, and the only benefit from prettier — automatic formatting. Now I propose to return to the original version with a common eslint config based on the @shower/core config.

@mrdimidium
Copy link

In the opposite direction, I really want to keep eslint — it is a static analysis tool, it protects against many potential errors. Prettier won't be able to replace it.

@shvaikalesh
Copy link

shvaikalesh commented Feb 12, 2019

@nikolay-govorov requires are getting sorted by eslint-plugin-import that came with Airbnb config, not Prettier.

@mrdimidium
Copy link

@shvaikalesh, Yes, you're right — I don't understand why this is happening yet, but it's not Prettier's fault.

@mrdimidium mrdimidium added the feature New feature or request label May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants