Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

What about a related project for @nodejs/automation projects utils? #11

Closed
alopezsanchez opened this issue Nov 2, 2017 · 19 comments
Closed

Comments

@alopezsanchez
Copy link

Hello.
I propose to create a project with common resources and utils for all the @nodejs/automation projects.

It would contains .editorconfig, eslint configuration files, gql queries and fixtures...
With this we get a centralized management of common resources and we avoid repeating files with the same content.

It could work like a dependency with a postinstall script that makes symlinks.

What do you think? I you give me the approval I start working on it.

Please comment your impressions! 😃

@tniessen
Copy link
Member

tniessen commented Nov 2, 2017

Are we already duplicating a lot? I didn't have a look at all the tools we are currently using, but I also did not come across a lot of duplicate files / contents yet. Duplicate APIs can always be extracted into npm packages, we don't even need to keep our dependencies within the org. GraphQL queries and fixtures are usually specific to an application, and those can still be extracted into npm packages if necessary. .editorconfig files are usually either very simple or very specific, plus they rarely ever change within a project. We could consider shared ESLint configuration files, but depending on who authored a package, the styles may differ a lot... Especially when it comes to custom rules.

Again, I don't know all of our automation / tooling code, so my impression might be wrong here.

@alopezsanchez
Copy link
Author

But, it could be a good practice to unify and apply the same coding style.. I dont't know...
Right now, we aren't duplicating a lot, but I think we need to look to the future and stablish some code and folder structure standards with the intention of make all the projects more readable to new collaborators.

Imagine this: in a hypothetical future we have code standards and 100 projects and we decide to change some ESLint rules, or indentation size or whatever. That means changing all the .eslintrc in all the repositories.

Maybe I'm rumbling too much. It's just an idea.

@Tiriel
Copy link
Contributor

Tiriel commented Nov 2, 2017

Although I'm all for some sort of "config standard" for all automation projects, I'd suggest we wait for Lerna and the monorepo to be properly set. We will then have a better vision of what we have, and what we need, on the deployment/release/config side of things.

Some of the tools we maintain or plan to maintain (like node-core-utils ) already check for standard configuration/commit messages, and standardization is in the core ADN of NodeJS I'd say.

But right now, I feel we are still in the phase of bootstrapping things, building a repo, and defining what to work on, and we don't know yet if it doesn't already exist in a form or another.

I don't know if I'm clear enough, sorry, it's late here and I'm french (irony).
Bottom line, tl;dr:
Great idea, but let's wait and see when monorepo's here. Let's first define our collaboration guidelines.

@alopezsanchez
Copy link
Author

@Tiriel Yes, you are being clear, but precisely for that reason I think we should stablish it ASAP.

PS: I'm spanish, we're neighbors 😂

@MylesBorins
Copy link

MylesBorins commented Nov 3, 2017 via email

@maclover7
Copy link

@MylesBorins there's a general discussion about automation projects at #1, I've got a PR open at #9 to move over the first tool and setup lerna

@joyeecheung
Copy link
Member

@MylesBorins there's a general discussion about automation projects at #1, I've got a PR open at #9 to move over the first tool and setup lerna

Yes I think this issue is similar to those discussions.

I like the idea of a monorepo for all the small utilities. I suggest we move on to #9 because there is already some work being done there. People can put a red corss if they don't like it, or discuss/put a green check if they do

@joyeecheung
Copy link
Member

Oh, on a second thought, we should probably have a issue dedicated to the monorepo discussion. We have not decided if we want a monorepo, is it going to happen in this repo or another repo (i.e. reserve this repo for discussion and documentations), so I think we can leave #9 alone for the moment.

@joyeecheung
Copy link
Member

Monorepo discussion thread opened in #12

@Tiriel
Copy link
Contributor

Tiriel commented Nov 3, 2017

Just to get back on the initial discussion then, it does seem logical for an automation team to have standards and automation tools...

I'm just not fond of doing everything at the same time, it feels like we may cut corner by doing so. But I'm with you on the need to do it on a very short notice.

@tniessen
Copy link
Member

tniessen commented Nov 3, 2017

in a hypothetical future we have code standards and 100 projects and we decide to change some ESLint rules, or indentation size or whatever. That means changing all the .eslintrc in all the repositories.

Keep in mind that the automation toolset is supposed to aid the development of the Node.js core, so creating dozens of automation-related projects maintained by the org is probably a bit far-fetched.

By the way, if you change a rule in the ESLint configuration and automatically sync it with all projects, you will also need to change the code in all of those projects, so you still need to manually work on every single project.

We can create repos all we want, but introducing additional dependencies (e.g. shared configs, APIs etc) comes with additional complexity, and complexity is something we should try to avoid.

I am not opposed to having a shared repository if it turns out to be necessary, but preemptively creating one does not seem like a good idea to me.

@Tiriel
Copy link
Contributor

Tiriel commented Nov 3, 2017

This is a valid point, but all these tools already need to work consistently on the same project (Node Core).
Following this, if we are to reduce complexity (for others), we should in effect try to bundle them in one loosely coupled super tool (while still allowing them to be used separately), and thus, having one coding standard form all of them really is necessary IMHO.

But I acknowledge that I may be thinking that way because I primarily come from PHP and specifically from Symfony, where standards are of paramount importance. So I'm totally open to suggestions.

I just think we would gain from a bit of standardization, while we do need to go progressively and wait for the initial setup to be stabilized a bit.

@phillipj
Copy link
Member

phillipj commented Nov 3, 2017

Even though code style is off topic in regards to the issue title, I do want to jump in early and propose using standard or similar like we do on the nodejs.org and github-bot codebases before anyone makes an effort in creating the "perfect eslint config" across many projects. I'm personally a big fan of not having to use time maintaining or discussing code style.

If that's very controversial, I think creating a dedicated issue for code style is better than going waaay off topic in this issue.

@tniessen
Copy link
Member

tniessen commented Nov 3, 2017

Side note: I don't think standard will get a lot of approval here, primarily due to the absence of semicolons and the presence of spaces after function names (had to deal with that yesterday when patching pino). I would prefer to stay close to the conventions we are using in Node.js core, considering that the majority of devs and users are probably core developers.

@Tiriel
Copy link
Contributor

Tiriel commented Nov 3, 2017

I don't think this is properly off-topic, since code-styling is part of what OP proposes via eslint files.

I do feel that keeping to a well established standard will be simpler. But I have to say, I too prefer using semi-colons, from time to time :)
And since these tools will be used in conjunction with core, keeping close to the styles used there seem a good idea.

@alopezsanchez
Copy link
Author

Maybe we could create another thread to define coding standars, or otherwise, use the Node.js ones.

@gibfahn
Copy link
Member

gibfahn commented Nov 3, 2017

Maybe we could create another thread to define coding standards, or otherwise, use the Node.js ones.

If the idea is to make tools for Node core, using the style Node core uses would make sense. I think the idea of standard is great, but (ironically) find the no-semicolons and spaces after function names causes way too much churn when converting projects.

before anyone makes an effort in creating the "perfect eslint config" across many projects.

node core does already have an eslint config, so we could just use that (or even extract it into a separate module).


To get back to the original issue, I think if we go with a monorepo approach, then a separate repo won't be necessary, and it seems like there is general agreement that a monorepo would be a good idea. So I'd suggest holding off on that.

@richardlau
Copy link
Member

node core does already have an eslint config, so we could just use that (or even extract it into a separate module).

👍 for extracting to a separate module. We could then use it in other foundation projects such as node-report (see nodejs/node-report#44).

@Trott
Copy link
Member

Trott commented Apr 22, 2023

Unarchived this repo so I could close all the PRs and issues. Will re-archive when I'm done.

@Trott Trott closed this as completed Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants