v2 Addon prep: move everything in to a single package monorepo #452
Conversation
NullVoxPopuli
force-pushed
the
to-monorepo
branch
from
942aa1f to
c6c200e
Compare
NullVoxPopuli
force-pushed
the
to-monorepo
branch
from
5819331 to
e4d073b
Compare
NullVoxPopuli
force-pushed
the
to-monorepo
branch
from
e4d073b to
791af86
Compare
NullVoxPopuli
force-pushed
the
to-monorepo
branch
from
549a075 to
c4c5483
Compare
| - uses: actions/checkout@v4 | ||
| - uses: wyvox/action-setup-pnpm@v3 | ||
| with: | ||
| node-version: 18.18.1 |
There was a problem hiding this comment.
@NullVoxPopuli looks strange we specify Node version only in this job, is that miss or really necessary?
There was a problem hiding this comment.
it's been needed until default node is this specific patch, because 18.17 and earlier was successful with webpack, but 18.18, (and 20+) had an issue with libuv, nodejs/node#49911
the specifying specifically, rather than relying on the volta config (or literally any other node version file), but action/setup-node (encapsulated by the wyvox action here) doesn't correctly support package.json as a "node version file" -- I've submitted a PR here: actions/setup-node#864 (comment) which should automatically propagate once merged and released, but it's been slow going getting anyone to look at (I don't really know who to ping)
| libpeerconnection.log | ||
| npm-debug.log* | ||
| testem.log | ||
| yarn-error.log | ||
|
|
||
| # ember-try |
There was a problem hiding this comment.
seem like all the below lines not needed any more and only copy what we have in addon/.gitignore?
There was a problem hiding this comment.
yeah, I'll address in a follow up
| @@ -43,7 +43,7 @@ const embroider = { | |||
|
|
|||
| module.exports = async function () { | |||
| return { | |||
| useYarn: true, | |||
| usePnpm: true, | |||
There was a problem hiding this comment.
IMO it's fine to bundle, maybe we could do this in separate PR. Which may slow down things considering this was idle for some time already.
There was a problem hiding this comment.
yeah, I'm with ya, it's just, it needs to happen anyway, and as as you state, this PR's been open for ages, I figured since both changes (move to single package monorepo, and pnpm) are pretty small, and internal changes, it's better to combine rather than configure ci.yml entirely differently twice for each of the changes, if split.
| "prettier": "^2.5.1", | ||
| "promise-polyfill": "^8.2.0", | ||
| "qunit": "^2.16.0", | ||
| "release-it": "^14.14.3", |
There was a problem hiding this comment.
not an action: in a follow up we may want to extract this to root level
There was a problem hiding this comment.
100% -- I have to finish release-it-plugins/workspaces#110 first -- thankfully we have a while before we'll want to release these changes, per: #458
| "lint": "pnpm --filter '*' lint:fix", | ||
| "start": "concurrently 'npm:start:*' --restart-after 5000 --prefix-colors cyan,white,yellow", | ||
| "start:addon": "pnpm --filter @ember/test-waiters start -- --no-watch.clearScreen", | ||
| "start:test-app": "pnpm --filter test-app start", |
There was a problem hiding this comment.
technically this is not yet ready, I believe we can keep as it will appear in next PR.
| @@ -0,0 +1,3 @@ | |||
| packages: | |||
| - addon | |||
| - test-app | |||
There was a problem hiding this comment.
would it cause any issues when workspace is missing?
|
This is great! Let's do it 🚀 ! |
Context, Plan, etc: https://gist.github.com/NullVoxPopuli/eafc7dad6547de5e730098498b829e1f
@ember/test-waitersspecific plan here: #458This PR moves the addon to a single package monorepo to prepare for a good few test apps for ensuring we keep compatibility as we prepare for v2-addon-ness.