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): Remove ReflectiveInjector symbol #48103

Closed
wants to merge 1 commit into from
Closed

refactor(core): Remove ReflectiveInjector symbol #48103

wants to merge 1 commit into from

Conversation

JeanMeche
Copy link
Member

@JeanMeche JeanMeche commented Nov 17, 2022 •
edited

ReflectiveInjector has been replaced since v5 and deprecated in v8, let's remove this.

  • Refactoring (no functional changes, no api changes)
  • Removing deprecated code

Does this PR introduce a breaking change?

  • Yes

What are the impacts at google ? Is this API still used ?

@JeanMeche JeanMeche force-pushed the chore/remove-ReflectiveInjector branch 5 times, most recently from 7538758 to 08d729b Compare November 17, 2022 13:45
@JeanMeche JeanMeche marked this pull request as ready for review November 17, 2022 14:09
@josephperrott josephperrott added the target: major This PR is targeted for the next major release label Nov 17, 2022
josephperrott
josephperrott approved these changes Nov 17, 2022
View reviewed changes
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

This is a breaking change and can't land until v16

Reviewed-for: benchpress

@dylhunn dylhunn added the area: core Issues related to the framework runtime label Nov 17, 2022
@ngbot ngbot bot added this to the Backlog milestone Nov 17, 2022
@dylhunn dylhunn modified the milestones: Backlog, v16-candidates Nov 17, 2022
@dylhunn dylhunn self-requested a review November 17, 2022 19:13
dylhunn
dylhunn approved these changes Nov 17, 2022
View reviewed changes
Copy link
Contributor

@dylhunn dylhunn 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: fw-core, fw-benchmarks

@dylhunn dylhunn removed the request for review from jessicajaniuk November 17, 2022 19:14
@dylhunn dylhunn added action: presubmit The PR is in need of a google3 presubmit core: di labels Nov 17, 2022
@alxhub alxhub added 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 labels Nov 18, 2022
@AndrewKushnir AndrewKushnir self-assigned this Mar 1, 2023
@AndrewKushnir
Copy link
Contributor

@JeanMeche could you please rebase this PR (and resolve conflicts) when you get a chance? We'll try to perform the necessary cleanup in Google's codebase by v16 to unblock this.

@JeanMeche JeanMeche force-pushed the chore/remove-ReflectiveInjector branch 3 times, most recently from 62d0be0 to 63d6ead Compare March 5, 2023 10:58
@AndrewKushnir
Copy link
Contributor

Global Presubmit.

@JeanMeche JeanMeche force-pushed the chore/remove-ReflectiveInjector branch from 63d6ead to 22ba849 Compare March 7, 2023 18:00
@AndrewKushnir
Copy link
Contributor

Presubmit #4.

jessicajaniuk
jessicajaniuk approved these changes Apr 4, 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

AndrewKushnir
AndrewKushnir approved these changes Apr 4, 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

@JeanMeche @AndrewKushnir
refactor(core): Remove ReflectiveInjector symbol
a4c02c3 
The `ReflectiveInjector` symbol has been deprecated in v5 (11 major versions ago). This commit removes ReflectiveInjector and related symbols.

BREAKING CHANGE: The `ReflectiveInjector` and related symbols were removed. Please update the code to avoid references to the `ReflectiveInjector` symbol. Use `Injector.create` as a replacement to create an injector instead.
@AndrewKushnir AndrewKushnir force-pushed the chore/remove-ReflectiveInjector branch from ab315d7 to a4c02c3 Compare April 4, 2023 21:41
@angular-robot angular-robot bot added the detected: breaking change PR contains a commit with a breaking change label Apr 4, 2023
@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 detected: breaking change PR contains a commit with a breaking change state: blocked action: global presubmit The PR is in need of a google3 global presubmit labels Apr 4, 2023
jessicajaniuk
jessicajaniuk approved these changes Apr 4, 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, circular-dependencies

@AndrewKushnir
Copy link
Contributor

Caretaker note: g3 cleanup is completed 🎉 This PR is now ready for merge (once CI run is completed). Please sync this PR into g3 as a separate change.

@AndrewKushnir AndrewKushnir changed the title refactor(core): Remove ReflectiveInjector refactor(core): Remove ReflectiveInjector symbol Apr 4, 2023
@dylhunn
Copy link
Contributor

dylhunn commented Apr 4, 2023

This PR was merged into the repository by commit 3b863dd.

@dylhunn dylhunn closed this in 3b863dd Apr 4, 2023
@JeanMeche JeanMeche deleted the chore/remove-ReflectiveInjector branch April 6, 2023 09:39
@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 May 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dylhunn dylhunn dylhunn approved these changes

@josephperrott josephperrott josephperrott approved these changes

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

@jessicajaniuk jessicajaniuk jessicajaniuk approved these changes

Assignees

@AndrewKushnir AndrewKushnir

Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime breaking changes core: di detected: breaking change PR contains a commit with a breaking change flag: breaking change merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: major This PR is targeted for the next major release
Projects
None yet
Milestone
v16-candidates
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@JeanMeche @AndrewKushnir @dylhunn @josephperrott @jessicajaniuk @alxhub