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

Resolve #132 #133

Merged
merged 2 commits into from Apr 12, 2022
Merged

Resolve #132 #133

merged 2 commits into from Apr 12, 2022

Conversation

@@ -54,7 +54,7 @@
"sinon": "^13.0.0"
},
"peerDependencies": {
"ember-cli": "~3.2.0 || ^4.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

@NullVoxPopuli shouldn't this be ^3.2.0 || ^4.0.0? I think there is no reason to prevent Ember CLI v3 users

Choose a reason for hiding this comment

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

The peerDependencies entry was added a long time ago, but was never released until v3.3.0.
Maybe the safest thing to do, is to completely remove it for now and then release a patch?
We can always re-add it later and release a new major just to be sure?

@bertdeblock
Copy link

@quaertym, mind having a look at this PR?
The current peer dependency statement breaks the ember new flow for NPM users. Specifically, the ~3.2.0 part.

@quaertym
Copy link
Owner

Let's go with ^3.2.0 || ^4.0.0 as suggested above.

@NullVoxPopuli
Copy link
Contributor Author

Is there a test somewhere to ensure this problem doesn't happen again?

@quaertym
Copy link
Owner

Not sure what's the best way to test this. But I think it is safe until ember-cli 5.x is released.

@NullVoxPopuli
Copy link
Contributor Author

I think, this would be a sufficient test to add in your CI:

ember new my-app --skip-npm
cd my-app
# remove line with your dependency
sed  '/"ember-cli-dependency-checker"/d' ./package.json
npm install
npm add --save-dev ember-cli-dependency-checker@file:../foo/bar/wherever
npm install

as long as everything has exit status code 0, that should be sufficient.

@quaertym quaertym merged commit ba8e765 into quaertym:master Apr 12, 2022
@quaertym
Copy link
Owner

@rwjblue any input on how to test this?

@quaertym
Copy link
Owner

BTW, I released this as v3.3.1.

@SergeAstapov
Copy link
Contributor

@quaertym I assume we can have ember-try scenarios with different versions of ember-cli, including latest/beta?

@rwjblue
Copy link
Collaborator

rwjblue commented Apr 12, 2022

Ya, using either ember-try or something like scenario-tester seems like the easiest setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants