search

machty / ember-concurrency

ember-concurrency is an Ember Addon that enables you to write concise, worry-free, cancelable, restartable, asynchronous tasks.
http://ember-concurrency.com
MIT License
691 stars 155 forks source link

Configure `@embroider/test-setup` #430

Closed alexlafroscia closed 3 years ago

alexlafroscia commented 3 years ago

This configures ember-concurrency to run the test suite against Embroider builds using the recommended approach (the @embroider/test-setup package).

This is an alternative to the approach taken in #368, which achieves the same goal manually; the package essentially does the same thing, but we abstract the "how" away to Embroider itself (which makes us more resilient to changes).

I configured the two Embroider jobs to allow failures, so that getting them to actually work can be done outside of the process of just seeing whether they pass or not.

alexlafroscia commented 3 years ago

Sorry that took me so long to get back around to! I rebased the PR to remove the merge conflict and removed the Embroider tests from running as part of CI.

alexlafroscia commented 3 years ago

Embroider 0.42.0 dropped yesterday, which no longer supports Node 10 (which this project uses). I couldn't bump this PR to use it because of the engines mis-match.

Is there a reason to support Node 10 these days, where the LTS release is now Node 14?

Maybe it would be good to (in a separate PR) update the support policy to 12/14/16 as Embroider is doing now?

Realistically the Node version policy is irrelevant for this addon, since there really isn't anything happening at the build-time level here.

maxfierke commented 3 years ago

Embroider 0.42.0 dropped yesterday, which no longer supports Node 10 (which this project uses). I couldn't bump this PR to use it because of the engines mis-match.

Is there a reason to support Node 10 these days, where the LTS release is now Node 14?

Maybe it would be good to (in a separate PR) update the support policy to 12/14/16 as Embroider is doing now?

Realistically the Node version policy is irrelevant for this addon, since there really isn't anything happening at the build-time level here.

heh, this is a bit of a philosophical debate I suppose. I personally don't have an issue bumping the Node version without a major version bump. We've done it before, we don't really hold ourselves to a specific one anyway, and I never know anything about any of the differences in the Node versions anyway. However, it's definitely come up before that people have had issues with that.

I think what I may do is merge this PR as-is and do some thinking about adding something of a policy in the README or somewhere else that indicates we consider Node version to be out-of-scope for SemVer given the general lack-of-bearing it has on ember-concurrency specifically.

Then we can do a bump to 0.42 separately once that's ironed out. (There's also a case to be made for tagging a version 3.0.0 as the next release as there's certainly some pre-Octane baggage we could probably start dropping in the next few months anyway, but there's maybe a couple more features I'd like to land in 2.x, so will maybe sit on that.)

alexlafroscia commented 3 years ago

Sounds good! I wasn't even necessarily thinking of the Node version bump not being a breaking change -- it's probably safer to make is a major bump, but as long as the CHANGELOG reflects that that's what broke, it's easy for people to determine if they can safely upgrade or if something needs to be done!