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

Fix(version): allow Angular major version other than 2 #3729

Closed
wants to merge 1 commit into from

Conversation

abdulhaq-e
Copy link
Contributor

I am not sure if other changes are required, but using angular v4 was blocked because of this strict version check. See this issue: #3720

@@ -105,11 +105,11 @@ export class Version {
if (v.isLocal()) {
console.warn(yellow('Using a local version of angular. Proceeding with care...'));
} else {
if (v.major != 2 || ((v.minor == 3 && v.patch == 0) || v.minor < 3)) {
if (v.major < 2 || ((v.minor == 3 && v.patch == 0) || v.minor < 3)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This would break at version 3.3.0!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But there will be no Angular 3, right? But you brought up a good point because this will fail in ng 4.3.0. I'll fix it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@abdulhaq-e are you still working on this? 4.3.0 would not work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just updated it, should work now.

@clydin
Copy link
Member

clydin commented Dec 28, 2016

Something to note is that this is probably intentionally strict to ensure that a private API is available.

@JohannesHoppe
Copy link
Contributor

The error message is implying a much more trivial reason...

@clydin
Copy link
Member

clydin commented Dec 28, 2016

Generally, implementation details are not exposed to the end user.

@JohannesHoppe
Copy link
Contributor

Sorry for stepping in lately.
But comparing versions is non-trivial and could lead to new errors in future.
We should use semver.compare instead of a error prone if-statement!

Please see this PR #3785

@abdulhaq-e
Copy link
Contributor Author

I agree, I thought about using it but because I am unfamiliar with angular-cli code I didn't want to add another library to the dependencies. It turns out it's already used!

I'll close this :) .

@abdulhaq-e abdulhaq-e closed this Dec 29, 2016
hansl pushed a commit that referenced this pull request Dec 30, 2016
@JohannesHoppe
Copy link
Contributor

I created a small fork for very impatient people:
https://github.com/angular-buch/angular-cli-ng4

It includes the required fix and has an updated ng new blueprint, too.

MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this pull request Feb 9, 2017
@abdulhaq-e abdulhaq-e deleted the fix_version_check branch March 20, 2017 19:50
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants