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

feat: add support for Angular v11 (follow up) #120

Merged
merged 11 commits into from
Nov 25, 2020
Merged

Conversation

JohannesHoppe
Copy link
Member

In retrospective, it wasn't my best idea to introduce the --configuration param. 😅

I think we want some kind of deprecation of our --configuration param since it's colliding with the designated param. We already have a breaking change due to the new stable deploy api. So let's clean everything up now.

See also
angular/angularfire#2281

follow up of: #118

FYI @beeman @dianjuar @mgechev

@beeman
Copy link
Contributor

beeman commented Nov 18, 2020

@JohannesHoppe I think it's a good call to clean it up! Happy to jump in to help, or test it locally!

@JohannesHoppe
Copy link
Member Author

JohannesHoppe commented Nov 18, 2020

Great, so let's enable the same kind of stuff like in angularfire. 🙂

I wonder what we do with people who now use ng deploy configuration=production. The correct command would then be ng deploy --buildTarget=production. But I cannot introduce an alias for a transition period because --configuration has a new meaning. Do you think it is sufficient to write the following text in red letters?

WARNING: The configuration option for specifying the BUILD has been renamed to buildTarget. Please change the parameter (eg. rename ng deploy --configuration=production to ng deploy --buildTarget=production). If you really want to adjust the DEPLOYMENT configuration, you can ignore this message.

Or do you have a better idea?

@beeman
Copy link
Contributor

beeman commented Nov 18, 2020

@JohannesHoppe I think that's a great solution to move away from the existing variable. Especially when you release it under a new major version folks can expect a breaking change :)

@dianjuar
Copy link
Contributor

@beeman I couldn't agree more.

  • Release those changes under a new major version
  • The warning that you mention on the changelog

I think that should be enough.

@dianjuar
Copy link
Contributor

dianjuar commented Nov 18, 2020

@JohannesHoppe I think I should do the same on ngx-deploy-npm, following up all your comments and links sounds the way to go.

... since 👨‍🚀 emoij is combined (🧑‍🚀) and breaks on some cmds
README.md Outdated Show resolved Hide resolved
src/deploy/schema.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@JohannesHoppe
Copy link
Member Author

🎉

Gentleman, I have pushed v1.0.0-rc.1 to NPM. Can you please give it a try and give me feedback if everything is fine?

I already left the tag to latest, so that it doesn't break for all people anymore.

many thanks to @fmalcher

Co-authored-by: Ferdinand Malcher <ferdinand@malcher.media>
@beeman
Copy link
Contributor

beeman commented Nov 25, 2020

Thanks a lot for publishing the RC @JohannesHoppe! I just did a test and it seems to work like a charm!

@JohannesHoppe
Copy link
Member Author

@beeman Thanks for the fast feedback. I will merge this PR to have the README up to date. I have some more ideas for rc2, so I would be happy for your continued support. 👍

@JohannesHoppe JohannesHoppe merged commit 6e492b9 into master Nov 25, 2020
@JohannesHoppe JohannesHoppe deleted the angular-11-v2 branch November 25, 2020 08:56
@beeman
Copy link
Contributor

beeman commented Nov 25, 2020

@beeman Thanks for the fast feedback. I will merge this PR to have the README up to date. I have some more ideas for rc2, so I would be happy for your continued support. 👍

For sure! I'm using it in several projects I'm currently working on so feel free to ping me when there is something to test 👍

@inpercima
Copy link

inpercima commented Nov 26, 2020

I just did a test too and it works. Thanks a lot. 👍

But I do got some warnings on installing with yarn.

warning " > angular-cli-ghpages@1.0.0-rc.1" has unmet peer dependency "@angular-devkit/architect@>= 0.900 < 0.1200".
warning " > angular-cli-ghpages@1.0.0-rc.1" has unmet peer dependency "@angular-devkit/core@^9.0.0 || ^10.0.0 || ^11.0.0".
warning " > angular-cli-ghpages@1.0.0-rc.1" has unmet peer dependency "@angular-devkit/schematics@^9.0.0 || ^10.0.0 || ^11.0.0".

Not sure why. The dependencies are installed.

@JohannesHoppe
Copy link
Member Author

🤔

@beeman
Copy link
Contributor

beeman commented Nov 26, 2020

I'm seeing the same. Here's a repro:

$ git clone git@github.com:beeman/angular-cli-ghpages-test.git
$ cd angular-cli-ghpages-test
$ yarn
yarn install v1.22.10
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
warning " > angular-cli-ghpages@1.0.0-rc.1" has unmet peer dependency "@angular-devkit/architect@>= 0.900 < 0.1200".
warning " > angular-cli-ghpages@1.0.0-rc.1" has unmet peer dependency "@angular-devkit/core@^9.0.0 || ^10.0.0 || ^11.0.0".
warning " > angular-cli-ghpages@1.0.0-rc.1" has unmet peer dependency "@angular-devkit/schematics@^9.0.0 || ^10.0.0 || ^11.0.0".
[4/4] 🔨  Building fresh packages...
✨  Done in 65.31s.

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.

5 participants