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

Prepare for V2 Addon conversion. #2027

Merged
merged 7 commits into from Oct 6, 2023

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Sep 23, 2023

Note the diff stats are way more intimudating than this PR actually is.
It's 90% "moves" + some boilerplate.
Very little has actually changed.


The primary purpose of V2 Addon conversions is to get ember apps installing deps, booting and building faster
(for me, anyway)


qunit-dom has never been compatible with modern packagers -- this is due to the issue reported in #2022 partially due to incorrect package.json config.

Since we want to also be good stewards of the ember ecosystem, we don't want to just fix the issue for non-ember users. We want to make sure that ember users don't pay the v1 addon / broccoli infrastructure cost when consuming this library.

Step one: convert to monorepo

  • extracting the tests to their own project
  • ensuring CI is up to date to be ok with the new structure
    Step two: convert the addon to v2
  • thankfully the library is already most of the way there -- this step will mostly be package.json tweaks
    • the package cross-compiles to IIFE (I presume for non-ESM CDN usage?) and ESM already.
  • this step will mostly be getting rid of the index.js, and other "emberisms"

For additional context on this process, I've written up this guide: https://gist.github.com/NullVoxPopuli/eafc7dad6547de5e730098498b829e1f

  • I've skipped the 1. Convert solo-repo to single-package monorepo step because I know that the MainMatter folks are more up to date with what's going on in the ember ecosystem than most, and the step is very small anyway.

Thankfully, qunit-dom is already using pnpm 🎉

Copy link
Contributor

@BobrImperator BobrImperator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine.
Some of the comments I added seem like they can be ignored since I made them before seeing #2028.

For consistency across our other repos, I'd like to ask to put each of the packages to /packages/ directory.
Other than that everything looks perfect 👍

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
@@ -30,7 +30,9 @@ jobs:

- name: auto-dist-tag
run: npx auto-dist-tag@1 --write
working-directory: qunit-dom
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here the working-directory makes sense to me, so I'd not change that.

Note; Although on another note we should consider setting up scripts for that in the root package.json.

package.json Show resolved Hide resolved
qunit-dom/package.json Show resolved Hide resolved
qunit-dom/package.json Outdated Show resolved Hide resolved
@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Oct 6, 2023

note, I will be discarding all package.json / lockfile changes because having a chain of 3 PRs to manage over a long period of time while upstream is changing so much is hard. (normally this wouldn't be the case, but the repo is getting restructured)

However, This PR: #2031
should be reviewed and merged first, as it's important for a changelog entry to happen before the v2 addon stuff.

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Oct 6, 2023

Also, I'm going to start squashing all my commits in each PR.
The entirety of #2028 is now in conflict (every file) (this is just the nature of re-structuring the repo upstream of changes 🤷 )
Though, it appears that the conflicts traverse through every commit, so...

I'm going to undo moving everything in to packages/* and we can change that after the PRs are merged.

This allowed me to be able to rebase #2028

…n-place

Restore workflows, unchanged

Put some documentation config / files back and get 'docs' building working

Looks like all of ci.yml should work now

Cleanup and get tests passing

Add lint:fix script

Get lints passing

Move renovate config back out to the root

release.yml should be up-to-date now as well

Restore ember-try config

Remove test:ember from the addon, because the test app has been extracted

Update ci.yml for compatibility testing

Move projects in to packages folder
@BobrImperator BobrImperator merged commit 8a486c4 into mainmatter:master Oct 6, 2023
9 checks passed
@NullVoxPopuli NullVoxPopuli deleted the v2-addon branch October 6, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants