search

stampit-org / stamp

Stamps - better OOP model
https://stampit.js.org
MIT License
25 stars 3 forks source link

Setup lerna for monorepo #1

Closed danielkcz closed 8 years ago

danielkcz commented 8 years ago

This is very preliminary PR, just to show the idea of how everything could be split apart. It's taken from current stampit@v3_0. I haven't touched code itself, only imports are changed. It uses Lerna as I've mentioned in https://github.com/stampit-org/stampit/issues/221#issuecomment-231522742

It's probably better to view code layout instead of changes: https://github.com/stampit-org/stamp/tree/setup-lerna

There is no linting, npm dependencies, tests and other stuff. I want to know if you like it before spending more time on it.

cc: @koresar

koresar commented 8 years ago

Hey, that's great. Could you please explain this repo purpose? What's the intention? What's the plan? Otherwise, it's hard to reason about the PR. Thanks ☺️

danielkcz commented 8 years ago

Well it's what I said in https://github.com/stampit-org/stampit/issues/221#issuecomment-231522742

So far I've split stuff to 4 packages:

Here we can have that most performant compose that would be used everywhere instead of copy&pasting it. Some for other function that were copied all the time.

All those packages would be published into separate scoped packages. The lerna takes care of publishing, versioning and even creates git tags. The actual stampit package can essentially use the chain as dependency and nothing will be broken for anyone. Obviously this isn't set in stone, it could be separated/named differently. I am just presenting core idea of whole thing.

koresar commented 8 years ago

Ok. This requires some discussion. I have no hard opinion on the project structure yet. There is something we need to include into that list though.

Sorry asking for these changes. But I believe all of them are good. I understand that it would need some work from your side.

Meanwhile, I believe this is a great PR and a great start. Love lerna, love the monorepo, amazing effort mate!

danielkcz commented 8 years ago

check-compose - the module which tests a pure compose implementation. I'd rather name it @stamp/check, and put the isSomething utilities into @stamp/is or alike.

Is it necessary to include check-compose in there too? I don't think it should be a goal to create a big pile of everything. Since this is command line utility, I think it can stay where it is. That said @stamp/is sounds indeed like better name for those small functions.

check-infected-compose - this should test infected compose implementation obv. Not sure if it should be a separate module or just an option of the @stamp/check module.

I think latter choice sounds better. Why to have two tools that do similar task?

When listing authors I believe it's time to be fair to all people. In other words, we better add all collaborators to the list of authors in package.json.

I agree, I just did not spent much time in there yet, it's really just beginning.

danielkcz commented 8 years ago

Ok I did some additional setup. Hopefully tomorrow I'll be able to add building and probably some basic tests for is, core packages. It should also run check-compose for both chain and compose packages.

I've also added contributors to readme, but I am not sure if I haven't left out someone. Apologies if I did.

I am not yet entirely sure about the chain package. As of now it's simple copy of https://github.com/stampit-org/stampit/blob/v3_0/src/stampit.js. Do we want it here actually? Or would there be only stampit with its code which would references these other 3 small modules?

It could be also somewhat half way as chain could be simply bunch of those helper functions like methods, statics, etc... (plus some coming from stamp-utils) and the stampit would glue it together with additional logic it has.

koresar commented 8 years ago

I'm not sure about the chain myself. It can be a simple stamp with pre-composed static methods, but not an infected compose. stampit is an infected compose as you know.

In other words:

stampit({init: function() {}, statics: {X: 1}); // valid
stampit().init(function() {}).statics({X: 1}); // valid
stampit({init: function() {}, statics: {X: 1}); // INVALID
stampit().init(function() {}).statics({X: 1}); // valid

So, if we introduce that @stamp/chain we'll confuse newcomers even more!

So, I ¯\_(ツ)_/¯

danielkcz commented 8 years ago

Ok,I've removed @stamp/chain as we agreed on and squashed commits. It's ready for merge now.