Skip to content

Fix beta checks correctly by using gt API#378

Merged
rwjblue merged 1 commit intoember-fastboot:masterfrom
kratiahuja:fix-version-checker
Apr 23, 2017
Merged

Fix beta checks correctly by using gt API#378
rwjblue merged 1 commit intoember-fastboot:masterfrom
kratiahuja:fix-version-checker

Conversation

@kratiahuja
Copy link
Contributor

Apparently satisfies does not conform to beta channel checks correctly.

> semver.satisfies('2.13.0-beta.1', '>=2.12.0-beta.1')
false

> semver.gt('2.13.0-beta.1', '2.12.0-beta.1')
true

cc: @rwjblue @danmcclain

@kratiahuja
Copy link
Contributor Author

Travis is failing. Not sure why. Will need to dig into this.

@rwjblue
Copy link
Member

rwjblue commented Apr 23, 2017

Ya, satisfies is trying to be "safe" with prerelease versions.

@kratiahuja
Copy link
Contributor Author

kratiahuja commented Apr 23, 2017

@rwjblue I forgot that this PR will introduce a minor regression. Earlier we allowed >=. With this PR we are moving to >. That's why the tests are failing. I see ember-cli-version-checker master has gte method which will make this absolute equivalent.

Is it possible to do a new release of ember-cli-version-checker? If not, then I can update the README to be a bit more explicit.

@rwjblue
Copy link
Member

rwjblue commented Apr 23, 2017

Ya, I can for sure, but you can also switch to beta.0 or alpha.0 for now...

@kratiahuja
Copy link
Contributor Author

Ohoo yeah I didn't realize that 😄

Apparently `satisfies` does not conform to beta channel checks correctly.

```js
> semver.satisfies('2.13.0-beta.1', '>=2.12.0-beta.1')
false

> semver.gt('2.13.0-beta.1', '2.12.0-beta.1')
true
```
@kratiahuja
Copy link
Contributor Author

@rwjblue This is good to merge. Travis is 💚 now. Thanks for your suggestion! 😄

@kellyselden
Copy link
Member

@kratiahuja I just published ember-cli-version-checker

@rwjblue rwjblue merged commit 18f0157 into ember-fastboot:master Apr 23, 2017
@rwjblue
Copy link
Member

rwjblue commented Apr 23, 2017

@danmcclain - have time to release?

@danmcclain
Copy link
Member

@rwjblue I can tonight

@kratiahuja
Copy link
Contributor Author

@danmcclain whenever you release, can you merge this one as well before releasing? #376

@danmcclain
Copy link
Member

Published v1.0.0-beta.18

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.

4 participants