Convert to Monorepo - #289
Conversation
Sorry, something went wrong.
FWIW, ember-cli-fastboot did the same general move recently (as did ember-engines). |
Sorry, something went wrong.
rwjblue
left a comment
There was a problem hiding this comment.
Some files should be moved back to the project root:
- CHANGELOG.md
- CONTRIBUTING.md
- RELEASE.md
Should add a top level version of these files:
- README.md (either duplicating or referencing the main packages/ember-cli-code-coverage/README.md)
Sorry, something went wrong.
| "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" | ||
| } | ||
| }, |
There was a problem hiding this comment.
Should be moved to root
Sorry, something went wrong.
| "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).
Sorry, something went wrong.
| @@ -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.
Sorry, something went wrong.
| 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?
Sorry, something went wrong.
| 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);
Sorry, something went wrong.
| "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
Sorry, something went wrong.
|
👍 Very much in favor of this layout, thanks for doing this! |
Sorry, something went wrong.
|
Updated to address the various review comments above. |
Sorry, something went wrong.
|
sorry, had it on the todo list, just didn't have time yet 😩 |
Sorry, something went wrong.
No worries! I was just helpin' out 😸 |
Sorry, something went wrong.
😱 😱 😱 😱
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-packages folder.
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 cd into the folder, run COVERAGE=true ember test there and manually verify the results in the coverage folder.
It also removes the need for running npm install during the test suite because the initial yarn install would 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.