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

RFC: Monorepo #323

Closed
ccpricenytimes opened this issue Nov 7, 2016 · 4 comments · Fixed by #330
Closed

RFC: Monorepo #323

ccpricenytimes opened this issue Nov 7, 2016 · 4 comments · Fixed by #330

Comments

@ccpricenytimes
Copy link
Contributor

Summary

This is a proposal to make kyt a monorepo with all the packages in one place. It will include separating kyt setup into a new tool called kyt-cli, which will allow users to more easily generate new kyt projects. It will also bring the NYT supported starter-kyts into the kyt repo.

Motivation

Separating kyt into multiple, local packages will benefit the project in the following ways:

  • Currently, starter-kyts are broken out into different repositories which has been a burden to maintain. In a monorepo, cross-cutting commits can be made to update changes in core and to any of the affected starter-kyts which will make them easier to maintain and version.

  • In its current form,kyt setup is limited as a utility to start new projects. Since kyt isn't globally installed, setup is mostly useful when a user wants to retrofit kyt into an existing project. However, it's not as useful when a user wants to setup a new project, including directory and package.json.

Many popular projects (eg.babel) are currently using the same monorepo approach.
Babel also has a great writeup on the benefits of monorepos.

Detailed design

Break up kyt into parts

kyt-cli

kyt-cli will be a globally installed package that allows users to create a new kyt project.

User flow:

  • npm install -g kyt-cli
  • kyt-cli setup
    Setup will prompt users with a list of the NYT supported starter-kyts to install. They will still be able to specify any github repo with the -r flag. kyt-cli setup -r https://github.com/user/cool-new-starter-kyt

The kyt-cli package will have its own testing and linting dependencies.

As part of this separation, it would be beneficial for setup to now install kyt as a devDependency. In order to do that, kyt start should be removed from kyt so that the kyt build system is not needed to run the app. Instead of including the current kyt start command in setup , npm run start should instead point to node build/server/main.js.

kyt-core

kyt core will contain the build sytem and tools needed to run kyt. It will still house the remainder of the cli including dev, build, run, test, lint, etc. This can be installed as a dependency of a project and used entirely on its own.

For reasons stated above, kyt start will be removed.

kyt-utils

kyt-utils will hold utitilies that will be shared by multiple packages. It will be published to npm as kyt-utils and referenced as a dependency in packages that need it. This package would not be used as a standalone module.

e2e tests

The e2e tests currently living in kyt will be pulled outside of the packages. All future e2e tests for any package should live here.

Move starter-kyts into kyt

NYT supported starter-kyts should be moved into kyt as packages. These starter-kyts include:

  • universal starter-kyt
  • static starter-kyt
  • test starter-kyt

A git migration tool should be used to maintain the history of these repos.

As part of this process the simple starter-kyt will be deprecated.

When setting up a new project kyt-cli setup will prompt the users with a list of the supported starter-kyts. If one is selected, simple-git will clone from kyt#master and copy the particular starter-kyt into the project.

Babel preset packages

There will be two babel preset packages that will also live in /packages. Details in the babel RFC.

Directory Tree

kyt
    /packages
        /kyt-cli
        /kyt-core
        /kyt-utils
        /starter-kyts
            / kyt-starter-universal
            / kyt-starter-static
            / kyt-starter-test
      /babel-preset-kyt-core
     /babel-preset-kyt-react
    /e2e-tests
    /docs

Testing / Linting / CI

The top level kyt directory will have its own package.json and linting and testing can be run from the top level, both locally and through Travis CI.

Publishing to NPM and maintenance

The parts of kyt will be published as separate npm packages:
kyt-cli --> kyt-cli
kyt-core --> kyt
kyt-utils --> kyt-utils

Drawbacks

One drawback for moving the starter-kyts into the kyt repo is the static starter-kyt will no longer be able to operate as a fork of universal. This means that when updating tools it will have to be done manually in two places.

Alternatives

The alternative is to keep kyt in one piece and continue to maintain the starter-kyts as separate repos.

Unresolved questions

Would it make sense to version the starter-kyts now that they are in one repo?
Should they be published as npm packages?

Other monorepo projects are using Lerna. Might be worth investigating.

@tizmagik
Copy link
Contributor

tizmagik commented Nov 11, 2016

LGTM 👍

  • One suggestion would be to inform users on kyt setup about the new package:
$ kyt setup

🐠 The setup command has moved to its own globally installable package, kyt-cli:

    npm install --global kyt-cli

  • Another thing to think about is that it would be nice to have kyt-cli check for updates to itself when it runs. Although I imagine it wouldn't be updated too much.
  • Also a warning about the setup command being deprecated would be helpful as well.

@ccpricenytimes
Copy link
Contributor Author

Thanks @tizmagik I think those suggestions make sense.

@felipesilva
Copy link
Contributor

What if we don't want to open source the kyt-starter-nytimes?

@tizmagik
Copy link
Contributor

@felipesilva that can still live in the internal private repo since setup supports passing in a repo URL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants