js-dxtools / webpack-validator

Validates your webpack config with Joi
MIT License
295 stars 29 forks source link

monorepo setup #64

Closed jonathanglasmeyer closed 8 years ago

jonathanglasmeyer commented 8 years ago

K guys, i went down the monorepo rabbit hole. :tada: Pretty much all the way adapted from the way babel does it. I need some solid review on this. :) cc @kentcdodds, @sarbbottam, @bebraw, @nyrosmith.

So, as discussed the disadvantage here is loosing semantic-release, ie we'll have to cut manual releases (like babel, react-router, react etc do it, just saying :)). This is sad because it's really nice to just hit a green button and have the next release pushed out by itself. But i think allow easy modularization will benefit this project and i just crave to now how it feels like to use the babel approach w/ lerna. We'll have to see how to handle semver here, i'm not entirely sure if the approach babel takes it really semver-like, as they increment the same version for all packages when there were changes in them since the last release point.

Anyways, looking forward to your thoughts.

nyrosmith commented 8 years ago

Can we change the name of the scheme module to the published one? Deep review will be done tomorrow. This looks quite complicated for what it tries to solve

kentcdodds commented 8 years ago

This looks quite complicated for what it tries to solve

An understatement IMO. I think the whole concept of a mono-repo is way outside of the scope of what we're trying to accomplish here... Babel and webpack-validator are in two totally different classes and I really don't think that the complexity here is adding any benefit.

But, like I said, I'm not doing much for this project, so feel free to ignore my opinion...

jonathanglasmeyer commented 8 years ago

I'm not really sure why you called it "scheme" because joi calls them "schema". I'd also like to have joi in the name because the schema depends on Joi. I'd suggest npm deprecate-ing the published package.

kentcdodds commented 8 years ago

I'm not really sure why you called it "scheme" because joi calls them "schema"

Apologies. Typo

I'd also like to have joi in the name because the schema depends on Joi. I'd suggest npm deprecate-ing the published package.

I'm fine with that...

nyrosmith commented 8 years ago

You change your mind very often @jonathanewerner. You told me there is no Problem in https://github.com/js-dxtools/webpack-validator/pull/63 and now I should deprecate it?

jonathanglasmeyer commented 8 years ago

@kentcdodds My last comment referred to @nyrosmith. @nyrosmith i said "so webpack-config-validationscheme already being released does not interfere with putting it in a monorepo." which is true, but does not imply that it is the best possible name for the contents of that package. :)

So, my reasoning here was complexity will be decreased even for 3-5 packages because integration tests are possible between multiple packages. But I'm not 100% sold either. :) I generally have the feeling that it sucks to have related code in different repos b/c testing/infra duplication & no integration, but it also sucks to have them in a monorepo because no semantic-release. meeeh. :/

jonathanglasmeyer commented 8 years ago

That being said, i'm totally open with going with a majority vote here, no hard feelings at all :) -- so far it seems like a 2:1.

bebraw commented 8 years ago

I trust your judgment on this. Sometimes there's no one right solution and no matter what you do, you lose something in the process.

How would releases work out exactly if we go with a monorepo?

nyrosmith commented 8 years ago

The scheme - schema thing was bc I did lots of DB schemes last week. Sorry for that. Avoiding Joi was by purpose cause if you ever use another lib for schema validation the lib name is screwed up.

ATM the monorepo setup makes no sense to me. If this is not a perfect example of fatigue and overcomplicating things I missunderstand the term itself.

jonathanglasmeyer commented 8 years ago

Ok, let's throw the branch away then. :) I'm half convinced and two people call over-engineering, decision done. :D I probably had to see it in front of me to really grasp the implications. Talking about it, there are some serious drawbacks of the lerna setup:

Should probably write about this or sth, these are some intriguing unsolved problems right there.

nyrosmith commented 8 years ago

@jonathanewerner I think this findings could give a very interesting Blog post.

nyrosmith commented 8 years ago

But the most important question: What's next?

jonathanglasmeyer commented 8 years ago

Ok, so let's see. I think Kent's suggestion is actually quite sensible:

I personally feel like doing this is overcomplicating things. But I haven't been a very active contributor lately, so feel free to ignore me. But honestly I think we could have two repos:

  1. The actual validator which exposes a node API to the raw schema as well as running a config through it. It can even log errors as it's doing today.
  2. The cli which utilizes the node API's raw schema to run a given config through and log errors (and fail a build potentially).

We need one core module doing validation and then a cli package on top of it. Even splitting out the joi schema was overengineered as the concept of extracting a schema implies that validation beyond this schema implies yet another package. One could think about a webpack-validator-core package -- but this would mean that webpack-validator package would actually be without any relevance, as it'd basically just call Joi.assert. Thus I now suggest:

This way we will have 2 packages, which is ok and hopefully not too overengineered. :)

Edit: Concerning point 2, "now" refers to the improved version from #63:

+    const validationResult = Joi.validate(config, schema_, { abortEarly: false })
+  if (validationResult.error) {
+    console.info(validationResult.error.annotate())
+    process.exit(1)
+  }

Maybe the side-effect free, validation-result-returning behaviour could be simply accomplished by supplying some kind of flag to the function.

kentcdodds commented 8 years ago

Yup :+1:

jonathanglasmeyer commented 8 years ago

I wrote about this: https://medium.com/@jonathanewerner/thoughts-about-package-modularization-d9631f7a41f1?source=linkShare-78849f7d88b4-1462611553