Open fsmanuel opened 1 year ago
@gilest tanks for the review!
Aren't there already unreleased breaking changes (node v12 dropped since 5.0)?
You are right! I missed that we haven't released https://github.com/adopted-ember-addons/ember-cp-validations/pull/731
My suggestion would be to also drop Node 16 and cut a major to avoid more rounds of breaking changes.
That sounds to be right. I think https://github.com/adopted-ember-addons/ember-cp-validations/pull/738 is a good place to drop Node v14.
Embroider
Question about the embroider suite failures – do you think they're genuine or to do with ember try and test app configuration?
I opened https://github.com/adopted-ember-addons/ember-cp-validations/issues/739 and https://github.com/adopted-ember-addons/ember-cp-validations/issues/740 to track the use of ember and ember-data internals. I suspect it to be the cause of the embroider failures.
Reason I ask is that I'm aware of a few apps building and running file using ember-cp-validations v5.
With v5 you mean ember v5?
Also a general suggestion to try and separate changes into more PRs. I know it's hard when no one is reviewing and there's lots of deferred maintenance like in this addon <3
You are absolutely right! So much better for the changelog and in general. I'm already confused what is released or not...
With v5 you mean ember v5?
I meant ember-cp-validations
v5.
What I was reporting is that it's running fine with ember 4.12 and full embroider.
I'm not sure if that's even useful as I've forgotten the context for this PR 😅
@mansona I rebased and this is what is left.
So I don't think we need any more of this PR so I think we can just close it 🤔 I opened all the other PRs with your work because they were all good and I didn't want you to have to go through the pain of rebasing after I changed things from under you 🙈. What's left here doesn't really help with anything, and the change that you make to import @ember-data/model
would require that to become an option peer dependency so if we do want to go down that route then we should open a PR to discuss it there 👍
I made an attempt to fix embroider stuff and it's getting us a lot further: https://github.com/adopted-ember-addons/ember-cp-validations/pull/749 🎉 We have a problem though 😞 it seems like you can't define computed properties the way that we're doing it here in the tests: https://github.com/adopted-ember-addons/ember-cp-validations/blob/master/tests/integration/validations/factory-general-test.js#L498
I don't know why this sort of issue would be unearthed by using Embroider and not by an ember upgrade 🤔 And unfortunately I don't have any idea how to change the test to get it to pass. Any thoughts?
@mansona I think it is related to https://github.com/adopted-ember-addons/ember-cp-validations/issues/739
Follow up on https://github.com/adopted-ember-addons/ember-cp-validations/pull/732
Model
from@ember-data/model
to detect if ember-data is installedisArray
on ember-data objects deprecationRelease, Beta and Canary are expected to fail. Current test coverage is
3.28
->4.12
I think we should release this as
v5.0.1
because it removes some deprecations and reduces the consuming apps noise.https://github.com/adopted-ember-addons/ember-cp-validations/issues/716