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.

Sign up for GitHub

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

Jump to bottom

refactor(core): deprecate NgProbeToken #51396

Closed
wants to merge 1 commit into from
Closed

refactor(core): deprecate NgProbeToken #51396

wants to merge 1 commit into from

Conversation

JeanMeche
Copy link
Member

This token serves no purpose on Ivy.

@JeanMeche JeanMeche added area: core Issues related to the framework runtime detected: deprecation PR contains a commit with a deprecation labels Aug 16, 2023
@ngbot ngbot bot modified the milestone: Backlog Aug 16, 2023
@angular-robot angular-robot bot removed the detected: deprecation PR contains a commit with a deprecation label Aug 16, 2023
@devversion
Copy link
Member

Please add a DEPRECATED: <x> description to the commit message so that it ends up showing up in the changelog

@JeanMeche JeanMeche force-pushed the chore/NgProbeToken branch 2 times, most recently from c684ec1 to c1b8075 Compare August 17, 2023 12:55
@angular-robot angular-robot bot added the detected: deprecation PR contains a commit with a deprecation label Aug 17, 2023
jessicajaniuk
jessicajaniuk approved these changes Aug 21, 2023
View reviewed changes
Copy link
Contributor

@jessicajaniuk jessicajaniuk 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 August 21, 2023 15:47
AndrewKushnir
AndrewKushnir reviewed Aug 21, 2023
View reviewed changes
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.

@JeanMeche could you please merge 2 commits into one?

Also, I'd propose to update the commit message:

refactor(router): deprecate the NgProbeToken

DEPRECATED:

The `NgProbeToken` is not used internally since the transition from View Engine to Ivy. The token has no utility and can be removed from applications and libraries.

@AndrewKushnir AndrewKushnir added target: major This PR is targeted for the next major release action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 21, 2023
@JeanMeche
refactor(core): deprecate the NgProbeToken
6858fc9 
DEPRECATED: NgProbeToken

The `NgProbeToken` is not used internally since the transition from View Engine to Ivy. The token has no utility and can be removed from applications and libraries.
AndrewKushnir
AndrewKushnir approved these changes Aug 21, 2023
View reviewed changes
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

@AndrewKushnir AndrewKushnir added action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Aug 22, 2023
@AndrewKushnir AndrewKushnir removed the request for review from atscott August 22, 2023 00:01
@AndrewKushnir
Copy link
Contributor

Presubmit.

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker action: global presubmit The PR is in need of a google3 global presubmit and removed action: presubmit The PR is in need of a google3 presubmit action: merge The PR is ready for merge by the caretaker labels Aug 22, 2023
@JeanMeche
Copy link
Member Author

@AndrewKushnir How did the presubmit go ?

@dylhunn dylhunn modified the milestones: Backlog, v17-final Oct 10, 2023
@dylhunn dylhunn self-assigned this Oct 10, 2023
@dylhunn
Copy link
Contributor

dylhunn commented Oct 11, 2023 •
edited

TGP: https://test.corp.google.com/OCL:572433273:BASE:572434576:1696991728973:803fffe2
Deflake: https://fusion2.corp.google.com/presubmit/572433273/OCL:572433273:BASE:572434576:1696998605685:825de75f/targets
Rerun at HEAD: https://fusion2.corp.google.com/presubmit/572433273/OCL:572433273:BASE:572457236:1697000236300:657e6d7e/targets

@dylhunn
Copy link
Contributor

dylhunn commented Oct 11, 2023

TGP reruns show a number of failures due to "Configured Target exited with: The target failed during analysis" -- looking at the logs, this seems to be some TAP issue, not conceivably related to this PR. Other than that TGP looks green, should be good to merge.

@dylhunn dylhunn 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: global presubmit The PR is in need of a google3 global presubmit merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Oct 11, 2023
@atscott atscott added target: rc This PR is targeted for the next release-candidate and removed target: major This PR is targeted for the next major release labels Oct 11, 2023
@dylhunn dylhunn 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 merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Oct 12, 2023
@dylhunn
Copy link
Contributor

dylhunn commented Oct 12, 2023 •
edited

As discussed at fw sync, this token is completely unused anywhere in 1P or 3P.

@pkozlowski-opensource
Copy link
Member

This PR was merged into the repository by commit a0dc12b.

pkozlowski-opensource pushed a commit that referenced this pull request Oct 13, 2023
@JeanMeche @pkozlowski-opensource
refactor(core): deprecate the NgProbeToken (#51396)
ba9fc24 
DEPRECATED: NgProbeToken

The `NgProbeToken` is not used internally since the transition from View Engine to Ivy. The token has no utility and can be removed from applications and libraries.

PR Close #51396
@pkozlowski-opensource
Copy link
Member

@JeanMeche thnx for this PR, it got merged into the repo. Additionally we've discussed it yesterday during the framework team sync and concluded that the NgProbeToken could be removed altogether since it doesn't appear to be used anywhere in G3.

If you could prepare a follow up PR with the NgProbeToken removal we could try to get it in before v17.

@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 Nov 13, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
refactor(core): deprecate the NgProbeToken (angular#51396)
c30e70a 
DEPRECATED: NgProbeToken

The `NgProbeToken` is not used internally since the transition from View Engine to Ivy. The token has no utility and can be removed from applications and libraries.

PR Close angular#51396
@JeanMeche JeanMeche deleted the chore/NgProbeToken branch February 29, 2024 13:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Assignees

@dylhunn dylhunn

Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime detected: deprecation PR contains a commit with a deprecation 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
Milestone
v17-final
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@JeanMeche @devversion @AndrewKushnir @dylhunn @pkozlowski-opensource @jessicajaniuk @atscott