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

refactor(forms): deprecate unwanted control events aliases #55698

Closed
wants to merge 1 commit into from

Conversation

JeanMeche
Copy link
Member

The types changed in the commit shouldn't have been aliased in the first place.

@pullapprove pullapprove bot requested a review from AndrewKushnir May 7, 2024 00:03
@JeanMeche JeanMeche added action: review The PR is still awaiting reviews from at least one requested reviewer area: forms labels May 7, 2024
@ngbot ngbot bot modified the milestone: Backlog May 7, 2024
@AndrewKushnir AndrewKushnir added action: global presubmit The PR is in need of a google3 global presubmit target: rc This PR is targeted for the next release-candidate labels May 7, 2024
@AndrewKushnir
Copy link
Contributor

AndrewKushnir commented May 7, 2024

Presubmit + TGP.

michael-small added a commit to michael-small/unified-form-control-theorycrafting that referenced this pull request May 7, 2024
JeanMeche added a commit to JeanMeche/angular that referenced this pull request May 8, 2024
Follow-up of angular#55698 to help remove the symbols from G3.
@JeanMeche JeanMeche force-pushed the fix/form-control-events branch 2 times, most recently from 28b7b04 to 57fe249 Compare May 8, 2024 01:13
@AndrewKushnir AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: global presubmit The PR is in need of a google3 global presubmit labels May 8, 2024
JeanMeche added a commit to JeanMeche/angular that referenced this pull request May 8, 2024
Follow-up of angular#55698 to help remove the symbols from G3.
@AndrewKushnir AndrewKushnir changed the title refactor(forms): remove Control events aliases refactor(forms): deprecate unwanted control events aliases May 8, 2024
@AndrewKushnir
Copy link
Contributor

Justification for this change:

  • this is an update to the feature that hasn't been released yet (will be a part of v18)
  • during the RC phase, we've identified that there is an inconsistency in event class naming (introduced accidentally by aliasing correct symbols with incorrect names)
  • this PR exposes correct symbols, while marking incorrect (existing) symbols as deprecated
  • this allows to do a cleanup in Google's codebase in places where incorrect symbol names are used
  • there is a followup PR that removes incorrect and deprecated symbols refactor(forms): remove deprecated symbols #55723

@pullapprove pullapprove bot requested a review from AndrewKushnir May 8, 2024 01:35
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.

Reviewed-for: public-api

@pullapprove pullapprove bot requested a review from atscott May 8, 2024 01:36
@AndrewKushnir AndrewKushnir requested review from thePunderWoman and removed request for atscott May 8, 2024 01:36
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir added this to the v18 final milestone May 9, 2024
@AndrewKushnir
Copy link
Contributor

@JeanMeche could you please rebase this PR and resolve merge conflicts when you get a chance?

@JeanMeche JeanMeche force-pushed the fix/form-control-events branch 2 times, most recently from 8fa4ebc to 13779a5 Compare May 10, 2024 01:21
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir removed the action: review The PR is still awaiting reviews from at least one requested reviewer label May 10, 2024
@AndrewKushnir
Copy link
Contributor

@JeanMeche the presubmit is "green" 👍 The test CI job is failing due to a pre-existing payload size issue (that was fixed in the main branch) and symbol golden files that require an update. Could you please update golden files when you get a chance? Thank you.

@AndrewKushnir AndrewKushnir added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: presubmit The PR is in need of a google3 presubmit labels May 10, 2024
This commit deprecates the aliases for the control events to ease the changes in G3
A follow-up commit will remove those deprecated entries.
@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker 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: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels May 10, 2024
@AndrewKushnir
Copy link
Contributor

Caretaker note: presubmit is "green", this PR is ready for merge.

@atscott atscott closed this in c8472e5 May 13, 2024
@atscott
Copy link
Contributor

atscott commented May 13, 2024

This PR was merged into the repository by commit c8472e5.

atscott pushed a commit that referenced this pull request May 13, 2024
This commit deprecates the aliases for the control events to ease the changes in G3
A follow-up commit will remove those deprecated entries.

PR Close #55698
JeanMeche added a commit to JeanMeche/angular that referenced this pull request May 13, 2024
Follow-up of angular#55698 to help remove the symbols from G3.
JeanMeche added a commit to JeanMeche/angular that referenced this pull request May 13, 2024
Follow-up of angular#55698 to help remove the symbols from G3.
JeanMeche added a commit to JeanMeche/angular that referenced this pull request May 14, 2024
Follow-up of angular#55698 to help remove the symbols from G3.
dylhunn pushed a commit that referenced this pull request May 17, 2024
Follow-up of #55698 to help remove the symbols from G3.

PR Close #55723
dylhunn pushed a commit that referenced this pull request May 17, 2024
Follow-up of #55698 to help remove the symbols from G3.

PR Close #55723
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker area: forms merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants