Skip to content

fix(forms): Make some minor fixups for forward-compatibility with typed forms#44540

Closed
dylhunn wants to merge 1 commit intoangular:masterfrom
dylhunn:tf-fixup
Closed

fix(forms): Make some minor fixups for forward-compatibility with typed forms#44540
dylhunn wants to merge 1 commit intoangular:masterfrom
dylhunn:tf-fixup

Conversation

@dylhunn
Copy link
Contributor

@dylhunn dylhunn commented Dec 20, 2021

Make the following fixes:

  • When submitting the entire migration in a disabled state, I commented out more tests than strictly required. It turns out it is possible to set the migration to optional by using a large version number, and then we can run the tests.
  • Responding to some final review comments caused two conditions to become flipped; fix these.
  • Always use explicit checks instead of boolean coercion.
  • Fix one missed any cast in a test case.

Targeting minor because the migration was merged into minor.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@dylhunn dylhunn added area: forms target: minor This PR is targeted for the next minor release labels Dec 20, 2021
@ngbot ngbot bot modified the milestone: Backlog Dec 20, 2021
@dylhunn dylhunn added the action: review The PR is still awaiting reviews from at least one requested reviewer label Dec 20, 2021
@dylhunn dylhunn force-pushed the tf-fixup branch 2 times, most recently from 0af4b7b to bf0f433 Compare December 20, 2021 22:50
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

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

LGTM (just one minor comment) 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use AbstractControl instead of any here (like we have it for the c2 form control)?

…ed forms.

Make the following fixes:
* When submitting the entire migration in a disabled state, I commented out more code than strictly required
* Responding to some final review comments caused two conditions to become flipped
* Always use explicit checks instead of boolean corecion
* Fix one missed any cast in a test case
@dylhunn dylhunn added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 4, 2022
@dylhunn
Copy link
Contributor Author

dylhunn commented Jan 4, 2022

This PR was merged into the repository by commit f7aa937.

@dylhunn dylhunn closed this in f7aa937 Jan 4, 2022
@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 Feb 4, 2022
@dylhunn dylhunn deleted the tf-fixup branch April 12, 2022 17:46
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: forms target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants