Monorepo #330

ccpricenytimes · 2016-11-23T20:23:37Z

fixes #323
fixes #327
Makes kyt a mono repo with several packages

kyt-cli - A globally installed package for project setup

setup command now can create a new project directory with the -d flag.
kyt-cli setup -d myDirectory
setup now supports copying devDependencies from starter-kyts

kyt-core - The kyt build and dev systems. Used as a project dependency

setup is now deprecated as part of kyt-core. It can be found in kyt-cli
start now runs the node server without a kyt wrapped command. This means kyt can be installed as a dev dependency

kyt-utils - Shared kyt code. Not to be used independently
starter-kyts - kyt-starter-static and kyt-starter-universal now live in the yt repo.

e2e tests have been pulled to the top level and will be used to test all packages.

Tasks before merge:

prepublish kyt-utils as alpha
point kyt-cli and kyt-core dependency to kyt-utils alpha rather than file dependency
Fix ToDos in setup

Post Merge:

Prepublish kyt-cli as alpha -- and test. Then remove hack

I've also started the docs which can get merged after all the monorepo packages are published.
https://github.com/NYTimes/kyt/compare/monorepo...monorepo-docs?expand=1

…s across packages

… double copied

…dependencies into dev dependencies

…dency version in starter kyt rather than kyt.version

jaredmcdonald

looks great overall! i have a bunch of microfeedback to take or leave

jaredmcdonald · 2016-12-09T15:40:12Z

.travis.yml

@@ -1,4 +1,4 @@
 language: node_js
 node_js:
  - 6.0
-script: npm run lint && npm test && npm run e2e
+script: cd packages/kyt-core && npm install && cd ../kyt-cli && npm install && cd ../kyt-utils && npm install && cd ../.. && npm run lint && npm install && npm test && npm run e2e


maybe worth breaking out into a separate script?

jaredmcdonald · 2016-12-09T15:40:48Z

CHANGELOG.md

+  - `setup` is now deprecated as part of kyt-core. It can be found in kyt-cli
+  - `start` now runs the node server without a kyt wrapped command. This means kyt can be installed as a dev dependency
+3. kyt-utils - Shared kyt code. Not to be used independently
+4. starter-kyts - kyt-starter-static and kyt-starter-universal now live in the yt repo.


typo, yt => kyt

jaredmcdonald · 2016-12-09T15:47:53Z

package.json

-  },
-  "engines": {
-    "node": ">=6.1.0"
+  "private": true,


what does "private": true mean here? and the version? maybe if this is just for the e2e tests, we could move those to another folder at the root level?

means it won't let you publish to npm, it's just as a safeguard. I think its nice to have these at the root level because you can run all the tests and linting at once.

jaredmcdonald · 2016-12-09T15:49:41Z

packages/kyt-cli/.eslintrc.json

@@ -0,0 +1,5 @@
+{
+  "extends": [
+    "../kyt-core/config/.eslintrc.base.json"


is the idea that this will change to a versioned dependency once kyt-core is (pre)published? thoughts on publishing the eslint configs separately too?

because this linting is just for when you're locally developing I think it's fine to point to another package. We can certainly revisit publishing the eslint configs. Thoughts @delambo?

sounds good!

jaredmcdonald · 2016-12-09T15:50:50Z

packages/kyt-cli/cli/actions/setup.js

-  userKytConfigPath,
-  userNodeModulesPath,
-  userPackageJSONPath,
-} = require('../../utils/paths')(); // eslint-disable-line import/newline-after-import
 // eslint-disable-next-line import/no-dynamic-require


I think you can lose the eslint comment now, it's not a "dynamic require" anymore

jaredmcdonald · 2016-12-09T15:53:09Z

packages/kyt-cli/cli/actions/setup.js

-  const tmpDir = path.resolve(userRootPath, '\.kyt-tmp'); // eslint-disable-line no-useless-escape
-  const repoURL = args.repository || 'https://github.com/NYTimes/kyt-starter-universal.git';
-  const removeTmpDir = () => shell.rm('-rf', tmpDir);
+  const tmpRepo = path.resolve(userRootPath, '\.kyt-tmp'); // eslint-disable-line no-useless-escape


this should just be equivalent to '.kyt-tmp' without the eslint comment, no?

jaredmcdonald · 2016-12-09T16:01:19Z

packages/kyt-cli/cli/actions/setup.js

+        type: 'list',
+        name: 'starterChoice',
+        message: 'Which starter-kyt would you like to install?', // eslint-disable-line
+        choices: ['Universal', 'Static'],


probably ok for now but it would be cool to have this assembled by just reading the contents of packages/starter-kyts, or at least some configuration somewhere?

also, maybe it's worth having a third option for "Specify a remote repository" where you don't necessarily need knowledge of the -r flag?

hmm yeah that sounds cool actually, maybe a post merge task?

jaredmcdonald · 2016-12-09T16:03:16Z

packages/kyt-cli/cli/actions/setup.js

+        {
+          type: 'confirm',
+          name: 'cliVersion',
+          message: 'There is a newer version of kyt-cli available. \n We recommend you upgrade before you continue. \n Would you like to proceed anyway?',


could this message output the installed and latest version of kyt-cli? (e.g. "You have 0.4.0 installed, the latest is 0.5.2")

yeah definitely! we can test after we prepublish

jaredmcdonald · 2016-12-09T16:08:24Z

packages/kyt-cli/cli/commands.js

+};
+
+
+program


another potential command this might have (just spitballing) could be ls-starter-kyts (or something with a better name) - lists available local starter kyts

i'd also add to the docs for the options (directory defaults to ., setup doesn't necessarily need a github url since it will prompt for static and universal, -r is a repo address for a starter-kyt)

ls-starter-kyts would involve finding the kyt repo and looking for the list because kyt-cli won't have any of that info locally, we should be thinking about how to keep all this connected though. I want to think through this a little more. Let's chat about it

ah ok, yeah, maybe there is a way to do this but let's not block this pr on it

delambo · 2016-12-09T16:49:52Z

CHANGELOG.md

+### BREAKING CHANGES
+
+#### MONOREPO
+kyt is now a mono repo with several packages


We should probably stay consistent on the name "monorepo"

delambo · 2016-12-09T17:07:39Z

packages/kyt-cli/package.json

+    "kyt-cli": "index.js"
+  },
+  "scripts": {
+    "test": "jest --testPathPattern /__tests__/",


Do we need test and lint here? I thought that we were testing and linting from the main /package.json.

i think the main package.json just runs the sub-packages' scripts

delambo · 2016-12-09T18:03:41Z

CHANGELOG.md

@@ -1,6 +1,24 @@

 ## Master

+## 0.4.0 (TBD)


er, shouldn't this go under the "## Master" section?

delambo · 2016-12-09T18:10:35Z

packages/kyt-cli/package.json

@@ -0,0 +1,32 @@
+{
+  "name": "kyt-cli",
+  "version": "0.0.0",


Start at 0.0.1

delambo · 2016-12-09T18:10:54Z

packages/kyt-utils/package.json

@@ -0,0 +1,21 @@
+{
+  "name": "kyt-utils",
+  "version": "0.0.0",


Start at 0.0.1.

…ep for publish after merge

* 'monorepo' of github.com:NYTimes/kyt: removing branch specific code and changing cli version to alpha to prep for publish after merge update kyt-utils to point to npm

* master: (35 commits) creates local copies of packages so e2e tests use latest code (#345) adds style and script linter packages (#344) now using npm version to check for kyt-cli updates (#337) Adds kyt version option to setup command (#343) make lint-script command; lint now runs scripts and style (#339) Adds bootstrap scripts (#341) fixes test coverage command (#342) Run test from root directory (#336) Monorepo (#330) [ci skip] document Jest/Watchman issue and common workaround (#334) Catch SIGINT (#332) Fixes e2e tests (#326) Removes deprecated reflect eslint rule (#325) Upgrade Linter dependencies (#289) [doc]fix path of eslintrc and stylelintrc (#321) Update unit tests for changes made to resolve #303 (#318) adds ISSUE_TEMPLATE (#317) Adds rfc template (#316) document possible kyt setup repository url formats (#314) Overwrites default npm init test script on setup (#293) ... # Conflicts: # .travis.yml # e2e_tests/tests/cli.test.js # package.json # packages/kyt-cli/cli/actions/setup.js

ccpricenytimes added 20 commits October 29, 2016 17:50

moving to packages

41b4e65

separating setup into kyt-cli; creating kyt-utils for shared utilitie…

be35691

…s across packages

set up linting and testing, check versions of packages in setup

18343c2

tests and e2e tests can now be run from root, eslint not working

a08f76e

moving node version check into a util

d1cc392

change dependency back to latest version from npm

deb15ad

Adding list selection of starter-kyts

6365996

updating start command

cb68175

moving kyt-starter-universal

5a5b6f7

copying starter-kyt

2c4f95f

adding starter-static, updating starter-universal name

0bd4a90

setup with static; currently kytconfig is getting installed twice...

896b09e

removing kyt.config.js from list of files, in static so it doesnt get…

96a97f6

… double copied

typo

cf39d5b

merging in master

ea90fd7

adding ability to create new directory as part of setup

e501d8f

updatinG top level lint commanD

5d21bd4

moving enzyme to devDependencies for testing

d1c9c46

lint

f00a57e

sync up eslint dependencies; changelog docs; moving some starter-kyt …

30debd5

…dependencies into dev dependencies

ccpricenytimes changed the title ~~Monorepo~~ WIP: Monorepo Nov 23, 2016

ccpricenytimes added 4 commits November 23, 2016 15:40

updating travis command

d67a1e5

more &&

72be9f3

Merge branch 'master' into monorepo

cc655d4

cleanup; kyt now gets added as a devDepency in existingProjectSetup

7ca1266

ccpricenytimes changed the title ~~WIP: Monorepo~~ Monorepo Dec 3, 2016

ccpricenytimes added 2 commits December 4, 2016 18:58

adds kyt start deprecation command

5700ddb

updating logging for setup; version check will now be based off depen…

023b684

…dency version in starter kyt rather than kyt.version

jaredmcdonald reviewed Dec 9, 2016

View reviewed changes

delambo reviewed Dec 9, 2016

View reviewed changes

ccpricenytimes and others added 4 commits December 9, 2016 13:40

code review feedback

b0ec1a7

Merge branch 'master' into monorepo

0dd30ea

prepping kyt-utils for prepublish

fcfe47c

update utils package.json

30faa6d

jaredmcdonald approved these changes Dec 9, 2016

View reviewed changes

ccpricenytimes and others added 5 commits December 9, 2016 14:43

update kyt-utils to point to npm

5b7a365

removing branch specific code and changing cli version to alpha to pr…

b8489c0

…ep for publish after merge

Merge branch 'monorepo' of github.com:NYTimes/kyt into monorepo

99d3401

* 'monorepo' of github.com:NYTimes/kyt: removing branch specific code and changing cli version to alpha to prep for publish after merge update kyt-utils to point to npm

moves package.json.scripts.lint to top level package.json

858ce72

eslint from kyt-core

3e8f51c

delambo merged commit 7385b0c into master Dec 9, 2016

delambo deleted the monorepo branch December 9, 2016 21:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Monorepo #330

Monorepo #330

ccpricenytimes commented Nov 23, 2016 •

edited

Loading

jaredmcdonald left a comment

jaredmcdonald Dec 9, 2016

jaredmcdonald Dec 9, 2016

jaredmcdonald Dec 9, 2016

ccpricenytimes Dec 9, 2016

jaredmcdonald Dec 9, 2016

ccpricenytimes Dec 9, 2016

jaredmcdonald Dec 9, 2016

jaredmcdonald Dec 9, 2016

jaredmcdonald Dec 9, 2016

jaredmcdonald Dec 9, 2016

ccpricenytimes Dec 9, 2016

jaredmcdonald Dec 9, 2016

ccpricenytimes Dec 9, 2016

jaredmcdonald Dec 9, 2016

ccpricenytimes Dec 9, 2016

jaredmcdonald Dec 9, 2016

delambo Dec 9, 2016

delambo Dec 9, 2016

jaredmcdonald Dec 9, 2016

delambo Dec 9, 2016

delambo Dec 9, 2016

delambo Dec 9, 2016

		};


		program

Monorepo #330

Monorepo #330

Conversation

ccpricenytimes commented Nov 23, 2016 • edited Loading

jaredmcdonald left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccpricenytimes commented Nov 23, 2016 •

edited

Loading