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

Using the eslint rules of node core for other projects in the foundation #55

Closed
joyeecheung opened this issue Jan 27, 2018 · 14 comments
Closed

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Jan 27, 2018

Hi, I have created these two npm packages that export the eslint rules of node core:

  1. eslint-config-node-core: Sharable ESLint configurations following the style guide of Node.js core.
  2. eslint-plugin-node-core: Custom ESLint rules from Node.js core.

Most of their sources are automatically generated from the source of the node core. I want to see how we can use these rules for other projects in the foundation. I believe with a unified coding style it would be easier for people to start contributing to them. AFAIK node-core-utils uses something that mimics the node core styles (but not quite), so I can start from there.

Action items:

  1. Move these two projects into the foundation, and move the npm packages to a scope that belongs to us (IIRC the @nodejs scope belongs to us? But I don't know who is the admin). We could also just use eslint-config-node-core or eslint-plugin-node-core (I've placed placeholders in both. My packages are currently published under @joyeecheung though), that way the config name would be shorter, just node-core, compared to @nodejs/eslint-config
  2. Find projects under the Node.js organization that are willing to switch to this coding style, and help them migrate.

What do you think?

@cjihrig
Copy link

cjihrig commented Jan 28, 2018

I'm in favor of doing this.

FWIW, @geek tried to do this a while back with a module named node-style. He ran into resistance because people didn't want to adopt core's style.

@mhdawson
Copy link
Member

mhdawson commented Jan 30, 2018

I think its a good idea as well. We might make it a requirement of moving a project under the foundation that the project will move to these standards over time.

It would be good to have linting enabled for node-addon-api if there is help to migrate.

@jasnell
Copy link
Member

jasnell commented Feb 17, 2018

Would it be better for this conversation to move to the @nodejs/automation WG repo?

@richardlau
Copy link
Member

FWIW, @geek tried to do this a while back with a module named node-style. He ran into resistance because people didn't want to adopt core's style.

For ref, nodejs/node-report#44.

@Trott
Copy link
Member

Trott commented Mar 28, 2018

I like having it as an option but I'm less excited about mandating it for all projects. Not strongly opposed to it. Just see it as providing potentially significant friction for relatively little benefit. I'd prefer we keep our requirements minimal. I could see requiring that new projects have lint rules in place. But they don't need to be the same across projects.

@mcollina
Copy link
Member

I would require linting to be in place. I would not force projects to change their linting rules because it's a very disruptive operation for the contributors.

@ljharb
Copy link
Member

ljharb commented Mar 28, 2018

I think it's also notable that core's style deviates in many ways from the more common idiomatic styles in the JS community; iow I agree with #55 (comment) and #55 (comment)

@joyeecheung
Copy link
Member Author

joyeecheung commented Mar 29, 2018

I like having it as an option but I'm less excited about mandating it for all projects. Not strongly opposed to it. Just see it as providing potentially significant friction for relatively little benefit. I'd prefer we keep our requirements minimal. I could see requiring that new projects have lint rules in place. But they don't need to be the same across projects.

I agree, this is more useful for projects that are directly related to core, e.g. projects that are vendored into/out of core, projects in deps/, and projects that are frequently used/maintained by collaborators e.g. llnode, node-core-utils

@gibfahn
Copy link
Member

gibfahn commented Mar 29, 2018

How about having it as a suggested default for core projects that don't already have a programming style, but not enforcing it?

@bnb
Copy link
Contributor

bnb commented Apr 2, 2018

@gibfahn rather than a suggested default, how about the default and core projects can deviate from that at their maintainers' discretion?

@Trott
Copy link
Member

Trott commented Apr 2, 2018

@gibfahn rather than a suggested default, how about the default and core projects can deviate from that at their maintainers' discretion?

That would present an unnecessary obstacle for moving existing projects into the org.

@Trott
Copy link
Member

Trott commented Jul 5, 2019

Closing stuff that has been inactive for more than a year in this repo, but if someone plans on picking this up, just go ahead and re-open! No strong opinions from me. Just tidying.

@Trott Trott closed this as completed Jul 5, 2019
@nschonni
Copy link
Member

@Trott since this has come up again recently, I wonder if this should get re-opened or just a new issue that is more targeted at creating a new config repo and taking the application as a separate issue.
//cc @nodejs/linting

@Trott
Copy link
Member

Trott commented Dec 23, 2021

@nschonni I like the open-a-new-issue approach.

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

No branches or pull requests