Skip to content

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

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

Closed

Conversation

devversion
Copy link
Member

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

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.
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 added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release risk: low labels May 14, 2019
@devversion devversion requested a review from a team as a code owner May 14, 2019 18:40
@ngbot ngbot bot modified the milestone: needsTriage May 14, 2019
@devversion devversion force-pushed the fix/static-query-improvements branch from 976961e to 83e1c6f Compare May 14, 2019 18:44
Copy link
Contributor

@kara kara left a comment

Choose a reason for hiding this comment

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

LGTM

@kara kara removed the action: review The PR is still awaiting reviews from at least one requested reviewer label May 14, 2019
@kara kara removed their assignment May 14, 2019
@kara kara added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit labels May 14, 2019
@kara
Copy link
Contributor

kara commented May 14, 2019

presubmit

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 fix/static-query-improvements branch from 83e1c6f to 08e00a6 Compare May 14, 2019 18:59
@kara kara added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: presubmit The PR is in need of a google3 presubmit labels May 14, 2019
@kara
Copy link
Contributor

kara commented May 14, 2019

merge-assistance: codefresh is lagging

@alxhub alxhub closed this in 6ceb903 May 14, 2019
alxhub pushed a commit that referenced this pull request May 14, 2019
#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 pushed a commit that referenced this pull request May 14, 2019
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 pushed a commit that referenced this pull request May 14, 2019
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 pushed a commit that referenced this pull request May 14, 2019
#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 pushed a commit that referenced this pull request May 14, 2019
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 pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
…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 pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
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 pushed a commit to BioPhoton/angular that referenced this pull request May 21, 2019
…#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
@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 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime cla: yes merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note risk: low target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants