adopted-ember-addons / ember-sortable

Sortable UI primitives for Ember.js
https://ember-sortable.netlify.app/
MIT License
298 stars 148 forks source link

Convert addon to V2 #508

Closed leoeuclids closed 1 year ago

leoeuclids commented 1 year ago

Why?

Ember addons in general are upgrading to the V2 structure, which requires any addons that depend on them to be V2 as well. Since this addon depends on ember-modifier, a V2 addon, upgrading it is blocked until this addon is also upgraded to V2. This also impacts apps that depend on ember-sortable and ember-modifier, directly on indirectly through other addons.

What it achieves

It will upgrade this addon to latest architecture of ember addons and also allow the upgrade of any dependencies to V2 addons.

Upgrade path

After this upgrade, apps that depend on this addon will have to make sure to use ember-auto-import >= 2, as well as upgrading any other addons that also depends on ember-modifier. That is one of reasons this change requires a major version bump.

What this PR does

Upgrades this addon to V2 by following the guide from Porting an Addon to V2.

Other things that should be done, not necessarily in this PR:

NullVoxPopuli commented 1 year ago

Since this addon depends on ember-modifier, a V2 addon, upgrading it is blocked until this addon is also upgraded to V2.

I think the beta , v4, is a v2 addon? would that work for this PR?

leoeuclids commented 1 year ago

Yeah, ember-modifier@4 (latest release) is a V2 addon and it works with this PR. I don't mind adding the upgrade here, just didn't do it to keep things separate.

NullVoxPopuli commented 1 year ago

yeah, since this repo (and all the adopted addons, afaik) use release-it, it'd be great you could separate that out as release notes are generated based on PRs <3

NullVoxPopuli commented 1 year ago

I wonder though, should we switch ember-modifier to be a peerDep?

if we do a breaking change release, then we can be a little more aggressive with the changes and a peerDep falls in line with "make the app/consumer choose which version to use"

leoeuclids commented 1 year ago

yeah, since this repo (and all the adopted addons, afaik) use release-it, it'd be great you could separate that out as release notes are generated based on PRs <3

I can do that PR right after this one lands.

I wonder though, should we switch ember-modifier to be a peerDep?

if we do a breaking change release, then we can be a little more aggressive with the changes and a peerDep falls in line with "make the app/consumer choose which version to use"

Sounds like a good idea to me.

leoeuclids commented 1 year ago

@NullVoxPopuli CI is failing on the Lint step, but I'm not sure why. Do you have any clues on how to fix it?

NullVoxPopuli commented 1 year ago

Looks like you need to run yarn and commit the changes?

leoeuclids commented 1 year ago

@NullVoxPopuli Could you approve the workflow task?

leoeuclids commented 1 year ago

@NullVoxPopuli It seems that CI is working for the most part, but I'm running into an issue with de 3.24 scenario, where it says that deprecate is not exported by @ember/debug, even though it was introduced on version 3.12. Running the scenario with ember >= 3.27 works fine.

Do you have any idea what could be causing this issue?

leoeuclids commented 1 year ago

It seems like V2 addons only support ember >= 3.28, so I'll create a new PR dropping support for the older versions, making it also appear on the changelog.

NullVoxPopuli commented 1 year ago

Ember addons in general are upgrading to the V2 structure, which requires any addons that depend on them to be V2 as well.

This premise isn't exactly true. Only requirement is ember auto import 2.

But anywho! Ready for rebase!

leoeuclids commented 1 year ago

This premise isn't exactly true. Only requirement is ember auto import 2.

Yeah, that makes more sense. Thanks for clarifying.

But anywho! Ready for rebase!

I'm new to open source, so let me ask about this. Should I just rebase this branch on main and force-push? Or should I pull from main and commit normally?

NullVoxPopuli commented 1 year ago

I'm new to open source,

most excellent! this is a fantastic early contribution!!! :tada:

Should I just rebase this branch on main and force-push? Or should I pull from main and commit normally?

yeah, rebasing on main is the way to go -- makes for manging your own changes and conflicts quite a bit easier.

my personal process is this:

# be on feature branch
git fetch origin
git rebase origin/main
# done! (assuming no conflicts, otherwise I work through conflicts and git rebase --continue)
# then:
git push <remote> <branch-name> --force-with-lease # light protection over force, 
#      in that you'll be warned in some scenarios if your remote has commits you 
#      don't have (useful for collaboration on the same branch)
leoeuclids commented 1 year ago

most excellent! this is a fantastic early contribution!!! 🎉

Thanks! 😄

yeah, rebasing on main is the way to go -- makes for manging your own changes and conflicts quite a bit easier.

Thank you for the explanation, I'll use that right away.

leoeuclids commented 1 year ago

@NullVoxPopuli There are 2 remaining tasks, but I'm not sure how tackle them.

  • [ ] Check release-it config
  • [ ] Deploy docs
NullVoxPopuli commented 1 year ago

can the docs be ran locally?

don't worry about release-it, I'll handle that <3

leoeuclids commented 1 year ago

can the docs be ran locally?

Yes, ember serve works inside the folder.