Convert to Monorepo#289
Convert to Monorepo#289rwjblue merged 6 commits intoember-cli-code-coverage:masterfrom Turbo87:monorepo
Conversation
FWIW, |
rwjblue
left a comment
There was a problem hiding this comment.
Some files should be moved back to the project root:
CHANGELOG.mdCONTRIBUTING.mdRELEASE.md
Should add a top level version of these files:
README.md(either duplicating or referencing the mainpackages/ember-cli-code-coverage/README.md)
| "changelog": { | ||
| "repo": "kategengler/ember-cli-code-coverage", | ||
| "labels": { | ||
| "breaking": ":boom: Breaking Change", | ||
| "enhancement": ":rocket: Enhancement", | ||
| "bug": ":bug: Bug Fix", | ||
| "documentation": ":memo: Documentation", | ||
| "internal": ":house: Internal" | ||
| } | ||
| }, |
| "release-it": { | ||
| "plugins": { | ||
| "release-it-lerna-changelog": { | ||
| "infile": "CHANGELOG.md", | ||
| "launchEditor": true | ||
| } | ||
| }, | ||
| "git": { | ||
| "tagName": "v${version}" | ||
| }, | ||
| "github": { | ||
| "release": true, | ||
| "tokenRef": "GITHUB_AUTH" | ||
| }, | ||
| "npm": { | ||
| "publish": false | ||
| } | ||
| } |
There was a problem hiding this comment.
Should be removed (in favor of the changes in ember-fastboot/ember-cli-fastboot#783).
| @@ -0,0 +1,67 @@ | |||
| 'use strict'; | |||
There was a problem hiding this comment.
I find it somewhat odd to have this file "loose" in the same directory as other packages.
| await execa('git', ['clean', '-f', 'my-app-with-in-repo-addon'], { cwd: __dirname }); | ||
| await execa('git', ['restore', 'my-app-with-in-repo-addon'], { cwd: __dirname }); |
There was a problem hiding this comment.
Seems vaguely odd to do git operations in tests, can you explain what this is cleaning up for?
| beforeEach(async function () { | ||
| await rimraf(`${BASE_PATH}/coverage*`); | ||
| await execa('git', ['clean', '-f', 'my-app'], { cwd: __dirname }); | ||
| await execa('git', ['restore', 'my-app'], { cwd: __dirname }); | ||
| }); | ||
|
|
||
| afterEach(async function () { | ||
| await rimraf(`${BASE_PATH}/coverage*`); | ||
| await execa('git', ['clean', '-f', 'my-app'], { cwd: __dirname }); | ||
| await execa('git', ['restore', 'my-app'], { cwd: __dirname }); | ||
| }); |
There was a problem hiding this comment.
This prelude seems to be repeated a bunch of times, can we make it a util and replace this in each file with:
setupTest(BASE_PATH);| "browser-test": "COVERAGE=true ember test", | ||
| "lint:hbs": "ember-template-lint .", | ||
| "lint:js": "eslint .", | ||
| "test": "npm run-script lint:js && npm run-script node-test" |
There was a problem hiding this comment.
This tries to run node-test, but that's gone
|
👍 Very much in favor of this layout, thanks for doing this! |
|
Updated to address the various review comments above. |
|
sorry, had it on the todo list, just didn't have time yet 😩 |
No worries! I was just helpin' out 😸 |
😱 😱 😱 😱
no, wait, hear me out!
This PR is inspired by the repository layout that https://github.com/embroider-build/embroider/ is using. Instead of using the barely maintained https://github.com/tomdale/ember-cli-addon-tests project, we instead use a monorepo layout with several fixture packages in a
test-packagesfolder.This enables us to use these test fixtures for debugging in a much easier way than before because they can also be used outside of running the test suite. In other words you can
cdinto the folder, runCOVERAGE=true ember testthere and manually verify the results in thecoveragefolder.It also removes the need for running
npm installduring the test suite because the initialyarn installwould have already installed all the necessary dependencies. Locally this seems to significantly speed up the test runs, for some reason they still seem to take roughly the same time on CI though.