shower / cli

Command line interface for Shower
Other
35 stars 6 forks source link

Inherit shower-core ESLint config #18

Open pepelsbey opened 5 years ago

pepelsbey commented 5 years ago

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 commented 5 years ago

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

pepelsbey commented 5 years ago

You could do that? :O

shvaikalesh commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

mrdimidium commented 5 years ago

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

shvaikalesh commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

mrdimidium commented 5 years ago

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

mrdimidium commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

mrdimidium commented 5 years ago

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