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(core): static-query migration should not fallback to test strategy #30458

Closed

Conversation

Projects
None yet
3 participants
@devversion
Copy link
Member

commented May 14, 2019

Various improvements for the static-query migration. See individual commits

devversion added some commits May 14, 2019

fix(core): static-query migration errors not printed properly
Apparently the devkit logger is not able to properly print
out error objects, so we need to convert them to a string
before in order to make the error visible to the user.

This is not testable without an e2e test that validates the CLI
terminal output.
fix(core): static-query migration should not fallback to test strategy
Currently if something fails in the selected strategy (e.g. AOT failures),
the migration currently accidentally falls back to the test strategy. This
is not helpful as we want to give developers the possibility to re-run
the migration after fixing potential AOT failures.

@devversion devversion requested a review from angular/fw-core as a code owner May 14, 2019

@ngbot ngbot bot modified the milestone: needsTriage May 14, 2019

@googlebot googlebot added the cla: yes label May 14, 2019

@devversion devversion force-pushed the devversion:fix/static-query-improvements branch from 976961e to 83e1c6f May 14, 2019

@kara

kara approved these changes May 14, 2019

Copy link
Contributor

left a comment

LGTM

@kara

This comment has been minimized.

Copy link
Contributor

commented May 14, 2019

refactor(core): improve messages for static-query migrations
Slightly improves the messages for the static-query migration in order
to make the terminal output less verbose but more helpful. Unfortunately
we are limited in what we can print due to the devkit not providing much
utilities for printing good messages from a migration schematic.

@devversion devversion force-pushed the devversion:fix/static-query-improvements branch from 83e1c6f to 08e00a6 May 14, 2019

@kara

This comment has been minimized.

Copy link
Contributor

commented May 14, 2019

merge-assistance: codefresh is lagging

@alxhub alxhub closed this in 6ceb903 May 14, 2019

alxhub added a commit that referenced this pull request May 14, 2019

fix(core): static-query migration should not fallback to test strategy (
#30458)

Currently if something fails in the selected strategy (e.g. AOT failures),
the migration currently accidentally falls back to the test strategy. This
is not helpful as we want to give developers the possibility to re-run
the migration after fixing potential AOT failures.

PR Close #30458

alxhub added a commit that referenced this pull request May 14, 2019

refactor(core): improve messages for static-query migrations (#30458)
Slightly improves the messages for the static-query migration in order
to make the terminal output less verbose but more helpful. Unfortunately
we are limited in what we can print due to the devkit not providing much
utilities for printing good messages from a migration schematic.

PR Close #30458

alxhub added a commit that referenced this pull request May 14, 2019

fix(core): static-query migration errors not printed properly (#30458)
Apparently the devkit logger is not able to properly print
out error objects, so we need to convert them to a string
before in order to make the error visible to the user.

This is not testable without an e2e test that validates the CLI
terminal output.

PR Close #30458

alxhub added a commit that referenced this pull request May 14, 2019

fix(core): static-query migration should not fallback to test strategy (
#30458)

Currently if something fails in the selected strategy (e.g. AOT failures),
the migration currently accidentally falls back to the test strategy. This
is not helpful as we want to give developers the possibility to re-run
the migration after fixing potential AOT failures.

PR Close #30458

alxhub added a commit that referenced this pull request May 14, 2019

refactor(core): improve messages for static-query migrations (#30458)
Slightly improves the messages for the static-query migration in order
to make the terminal output less verbose but more helpful. Unfortunately
we are limited in what we can print due to the devkit not providing much
utilities for printing good messages from a migration schematic.

PR Close #30458

BioPhoton added a commit to BioPhoton/angular that referenced this pull request May 21, 2019

fix(core): static-query migration errors not printed properly (angula…
…r#30458)

Apparently the devkit logger is not able to properly print
out error objects, so we need to convert them to a string
before in order to make the error visible to the user.

This is not testable without an e2e test that validates the CLI
terminal output.

PR Close angular#30458

BioPhoton added a commit to BioPhoton/angular that referenced this pull request May 21, 2019

fix(core): static-query migration should not fallback to test strategy (
angular#30458)

Currently if something fails in the selected strategy (e.g. AOT failures),
the migration currently accidentally falls back to the test strategy. This
is not helpful as we want to give developers the possibility to re-run
the migration after fixing potential AOT failures.

PR Close angular#30458

BioPhoton added a commit to BioPhoton/angular that referenced this pull request May 21, 2019

refactor(core): improve messages for static-query migrations (angular…
…#30458)

Slightly improves the messages for the static-query migration in order
to make the terminal output less verbose but more helpful. Unfortunately
we are limited in what we can print due to the devkit not providing much
utilities for printing good messages from a migration schematic.

PR Close angular#30458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.