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

Move utils and components into this repo #109

Closed
necolas opened this issue Sep 9, 2015 · 41 comments
Closed

Move utils and components into this repo #109

necolas opened this issue Sep 9, 2015 · 41 comments

Comments

@necolas
Copy link
Contributor

necolas commented Sep 9, 2015

SUITCSS could be developed as a monorepo to easy versioning and toolchain setup. This wouldn't stop us being able to publish to the individual npm packages.

@simonsmith
Copy link
Member

Are you thinking releasing SUIT as an entire package each time a change is made, instead of per component/util? Would definitely ease issue reporting and PRs I expect

@necolas
Copy link
Contributor Author

necolas commented Sep 10, 2015

Yeah. And should make development easier in general. It's a suggestion, as I won't be able to put time into it myself. Just to say that I'm not militant about the multi-repo approach.

@simonsmith
Copy link
Member

Sounds like a sensible idea to me. I feel it could even remove the need for separate npm packages, just encourage users to npm install suit and then import the components/utils needed from within. If any of them change then publish a new version of whole thing.

Kind of similar to how it works now I suppose, where it has base, utils and components as dependencies

@timkelty
Copy link
Member

I feel it could even remove the need for separate npm packages, just encourage users to npm install suit and then import the components/utils needed from within.

@simonsmith My only concern there is always having to upgrade all changes. E.g. if you wanted the latest compontents-grid but didn't want to mess with anything else.

In reality, probably not a huge deal, but if the amount and/or complexity of components grow in the future, maybe something to consider.

@simonsmith
Copy link
Member

If we had all the components within suit how would we publish separate releases of each? I know you can use dist-tags in npm but don't you still need to point it at a single git tag? If it was all under one repo then it could get messy.

@oleersoy
Copy link
Contributor

One thing I find refreshing about the individual repositories is that it's easy to get up to speed fast, since there is less information to cover. If you want to get up to speed by reading through issues in order to understand the design, it's less intimidating than having one monolithic project. There are also less files to wade through when understanding the build, tests, etc.. More here:
https://github.com/substack/browserify-handbook#module-philosophy

Having one to one correspondences between the NPM package and the corresponding github repository is what most of us are used to. The README for the NPM package corresponds to the README at the root of the github repository, etc.

@timkelty
Copy link
Member

I could go either way on multi-repo vs. one, but I think the npm packages should definitely stay separate.

@simonsmith
Copy link
Member

So as @necolas says in this PR, managing a PostCSS build system across so many different repos is going to be a real hassle. For example, to work on SUIT I had to run a script to checkout the entire org, which is just working round not having everything in one repository.

Based on that I'd be keen to move this issue along. Some points to clarify:

  • All the packages remain on npm as separate. I think this makes a lot of sense.
  • Should we do away with per-package build scripts? Might make sense as usually if you want just a couple of packages you have a build step already in place. For those wanting to build the whole SUIT library then cloning this repo and running npm run build will do the job.
  • Do we still need the shortcut packages components and utils in this setup?
  • How do git tags work within this repository once it's all under one roof? Should we just push a new version any time one of the packages changes, as well as maintaining the npm tags of each package?

@giuseppeg
Copy link
Member

I started a local branch already to fix this issue.

I am organizing the folders as follow:

.
├── doc
├── base
|   └── index.css
├── component
|   ├── arrange
|   |   └── index.css
|   ├── grid
|   └── ...
├── util
|   ├── text
|   ├── size
|   └── ...
├── theme
...
└── ...

Should everything go under a lib/ folder?
What do you think about using the singular form for the folders names?

@import 'component/arrange';
@import 'util/text';
Do we still need the shortcut packages components and utils in this setup?

Maybe, TBD.

All the packages remain on npm as separate. I think this makes a lot of sense.

Yes.

Should we do away with per-package build scripts? Might make sense as usually if you want just a couple of packages you have a build step already in place. For those wanting to build the whole SUIT library then cloning this repo and running npm run build will do the job.

Yes, it makes sense since they are published on npm as separate.

How do git tags work within this repository once it's all under one roof? Should we just push a new version any time one of the packages changes, as well as maintaining the npm tags of each package?

We'd tag suit as a whole (versions are free : ) and maintain the npm tags of each package.

@simonsmith
Copy link
Member

Looks good @giuseppeg

What do you think about using the singular form for the folders names?

I'd vote to keep it components just to align with npm, which is hopefully where most people will install from anyway

@simonsmith
Copy link
Member

Should everything go under a lib/ folder?

I don't think this is necessary.

Are you adding the PostCSS build steps in this branch?

@giuseppeg
Copy link
Member

Are you adding the PostCSS build steps in this branch?

To each single package? I could or maybe it is better to just make the monorepo and refactor the build in a followup.

@simonsmith
Copy link
Member

Ah yeah, sorry I didn't mean to each package. That would be quite a chore. Yeah, let's do the PostCSS switch as a separate task.

@giuseppeg
Copy link
Member

I'd vote to keep it components just to align with npm

Would you strip it from the pkg name?
components/component-arrange/ vs components/arrange/

@simonsmith
Copy link
Member

I think your idea of components/grid, components/arrange etc is good 👍

@giuseppeg
Copy link
Member

Should the parent packages suit, components and utils depend on the local ones

"dependencies": {
    "suitcss-components-grid": "file:./components/grid"
}

for example — or the ones published on npm?
If so how do we publish them to npm?

@simonsmith
Copy link
Member

Hmm, good question.

I suppose one route is to bring back lib:

components
├── README.md
├── index.css
├── lib
│   ├── arrange
│   ├── button
│   └── grid
└── package.json

That would enable lib to be ignored by git which npm would also respect, then components could still be published and so could the individual component directories.

@timkelty
Copy link
Member

timkelty commented Oct 8, 2015

Do we still need the shortcut packages components and utils in this setup?

Since we're moving to a PostCSS ecosystem, that leads itself to using postcss-import, in which a user could do:

@import 'suitcss/components/**/*.css'
@import 'suitcss/utils/**/*.css'

So I vote to drop the shortcuts ('utils', 'components') Plus maintaining them could be a chore.

@timkelty
Copy link
Member

timkelty commented Oct 8, 2015

Hmmmm...it sounds like publishing multiple npm packages from a single repo is not as easy as I thought it might be: npm/npm#2974

Anyone have an idea how we planned on proceeding?

This wouldn't stop us being able to publish to the individual npm packages.

@necolas I'm pretty sure I've seen other repos do this, but I think it requires some build script magic. Do you have any examples?

@Florian-R
Copy link

@timkelty

I'm pretty sure I've seen other repos do this, but I think it requires some build script magic. Do you have any examples?

Lodash does it, also, it's worth to take a look to the amp project.

@simonsmith
Copy link
Member

I think @timkelty's idea could work there. postcss-import supports globbing and looks in node_modules by default.

It's also probably good practice to encourage users to install what they need

@simonsmith
Copy link
Member

@timkelty @giuseppeg Added postcss as the build task in the wip branch. It raises a couple questions:

  • Should the top level index.css include custom media properties to stop the media queries for the size utils being stripped from the output?
  • What happens to the test pages if we take away per package build tools and the gh-pages branches? Do we consolidate this under a single page on the SUIT site?

@giuseppeg
Copy link
Member

@simonsmith awesome looks good, thank you!

From now on I suggest that we branch off suitcss/suit/tree/monorepo when we want to try new things and merge or patch it when it makes sense i.e. use the wip branch as "master" and the experiments as feature/patch branches.

stop the media queries for the size utils being stripped from the output

This is not a new issue right? If so I wouldn't try to solve it here.

What happens to the test pages if we take away per package build tools and the gh-pages branches? Do we consolidate this under a single page on the SUIT site?

Each package should still have a build script and tests in my opinion (like you did in suitcss/components-button#19)

Now that I think about this maybe we shouldn't use local paths in package.json and revert my change.

We could add a test.css file at the root of the project and import the local files.
Before releasing a new version we would preprocess test.css to make sure that nothing breaks.

@timkelty
Copy link
Member

Nice work, @simonsmith.

My first thought is we might want base, components, utils, theme nested under a folder. Could be lib (following other suit repos), or css or whatever. Just to keep the actual guts of the project separate from say, doc.

What do you all think?

@timkelty
Copy link
Member

I thought the build and test tooling was going to be handle at the top level in the mono-repo, not per-package.

If we don't do that, I'm not sure what we're gaining by going with the mono-repo approach? (feel free to remind me :))

@giuseppeg
Copy link
Member

@timkelty with the mono-repo you can make a change to all the README.md for example and make just one commit instead of having to clone n repos and do it one by one.

Considering that we are going to publish the single packages to npm, each pkg still needs a build script and tests because it is standalone.

@simonsmith
Copy link
Member

From now on I suggest that we branch off suitcss/suit/tree/monorepo when we want to try new things and merge or patch it when it makes sense i.e. use the wip branch as "master" and the experiments as feature/patch branches

I'm not really fond of the develop branch style if it doesn't add much. Pushing a feature branch whenever needed seems to work okay.

This is not a new issue right? If so I wouldn't try to solve it here.

You're correct, but I always wondered if it was a bug and it's even been reported before. It does start to step on the toes of theme but it does seem odd for a load of console warnings to be normal.

With regard to the per package build scripts, I did also raise about per package build scripts earlier in this issue and it sounded like the idea would be to ditch them. @necolas also pointed out that it will be a chore to maintain the pipeline across so many repositories. I think one of the big wins of a single repo was to keep the build script in one place, and if people want to use individual packages then it's very likely they'll just import them into an existing postcss pipeline. If not, we can certainly encourage this.

It may not be feasible with the need for testing the components though, so not sure how to proceed on this. Losing the gh-pages branches kind of destroys this too, even with per package build scripts in place

@giuseppeg
Copy link
Member

I'm not really fond of the develop branch style if it doesn't add much.

It keeps the git history clean, no need to revert if we reconsider some choices. Not a big deal though ;)

and if people want to use individual packages then it's very likely they'll just import them into an existing postcss pipeline.

Agreed!

I'll try to think of a way to keep the tests on the repo.

@simonsmith
Copy link
Member

It keeps the git history clean, no need to revert if we reconsider some choices

I am equally OCD about a good history :)

Won't we be all good if we keep stuff in branches, tested and agreed before bringing it into master, rather than a git-flow style system?

In terms of the tests, I guess ultimately they are no more than just static pages. We could just generate them for each repo with a build step and then they can be viewed locally. They won't be public pages on gh-pages but maybe we could instead create a more user-friendly page on the SUIT website that had examples of each component and utility.

@giuseppeg
Copy link
Member

Build them as static files sounds good, although that may not necessarily reflect their actual state when the user customizes em (eg. defines custom mq, gutters etc.) <- maybe a non-problem though.

@simonsmith
Copy link
Member

What do you all think?

Yeah, sounds like a good idea @timkelty. I'm down with lib.

@simonsmith
Copy link
Member

Done in latest commit. I figure this might make it easier to use things like glob as well.

@simonsmith
Copy link
Member

Just to update on this, I think we're heading along the route of keeping the repositories as is, and re-factoring the preprocessor to use postcss. This was off the back of a spike @giuseppeg has spent some time on.

More info on this in this issue - suitcss/preprocessor#11 (comment)

Here is the PR for reference too - suitcss/preprocessor#12

@timkelty
Copy link
Member

Just ran across this, which was interesting: https://github.com/babel/babel/blob/master/doc/design/monorepo.md

@simonsmith
Copy link
Member

Oh nice find. Yeah, I think it's a smart way to go from the start.

@timkelty
Copy link
Member

Yeah - I still think it is an attainable migration for SUIT, but right now I'd rather just plow away with the packages/repos as is.

As far maintating support for bower/component with such a move...

I don't really think this should be a concern. IMO bower being "dead" is more or less true, and it is always still possible to install any arbitrary repo with bower. Furthermore, our other npm packages, while deprecated, wouldn't be going anywhere.

With the PostCSS migration, we're expecting/encouraging users to have thier own postcss toolchain in place anyhow, so there's really no need for bower (no need to have the suit source css in a public web root).

@oleersoy
Copy link
Contributor

In superfly-css I've implemented a standard directory layout superfly-css-pli, along with self contained gulp tasks (Example superfly-css-task-build ), that I think reduce the weight of, if not eliminate, most of the arguments cited by babel for maintaining a monorepo. Simple focused repositories are easier to manage and get up to speed on from a dependency, core concern / boundary, and information volume standpoint.

@simonsmith
Copy link
Member

I personally think giving up the issue and git commit history is too big a sacrifice just on its own, basically destroys git blame and the tags too.

I can 100% see the value in it from the start of a project though.

@giuseppeg
Copy link
Member

Should we close this issue?
cc @simonsmith @timkelty

@simonsmith
Copy link
Member

Yep

@timkelty
Copy link
Member

Yep!

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

6 participants