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 signal mutate implementation #52348

Closed
wants to merge 1 commit into from
Closed

refactor(core): remove signal mutate implementation #52348

wants to merge 1 commit into from

Conversation

JeanMeche
Copy link
Member

signal.mutate has been removed but the implementation was kept somehow. It's not used.

@JeanMeche JeanMeche marked this pull request as ready for review October 24, 2023 14:15
@pullapprove pullapprove bot requested a review from crisbeto October 24, 2023 14:15
@crisbeto crisbeto requested review from pkozlowski-opensource and removed request for crisbeto October 24, 2023 15:07
@garrettld
Copy link
Contributor

With this change, the signalValueChanged function is now only called in one place. Should it be inlined?

@pkozlowski-opensource
Copy link
Member

With this change, the signalValueChanged function is now only called in one place. Should it be inlined?

I would keep it as-is for readability. JS engines will optimize it as needed.

@pkozlowski-opensource pkozlowski-opensource added state: blocked area: core Issues related to the framework runtime labels Oct 25, 2023
@ngbot ngbot bot added this to the Backlog milestone Oct 25, 2023
@pkozlowski-opensource
Copy link
Member

@JeanMeche we still have quite a bit of usage of this function in G3 and will need to remove those before this PR can get in.

@swami-sanapathi swami-sanapathi mentioned this pull request Oct 29, 2023
Closed
@pkozlowski-opensource pkozlowski-opensource 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 state: blocked labels Jan 5, 2024
@pkozlowski-opensource
Copy link
Member

@JeanMeche the relevant cleanups were done in G3 - could you rebase this PR so we can get it in?

@JeanMeche JeanMeche added action: review The PR is still awaiting reviews from at least one requested reviewer 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 Jan 5, 2024
@pkozlowski-opensource pkozlowski-opensource added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 9, 2024
@atscott atscott added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jan 9, 2024
atscott
atscott approved these changes Jan 9, 2024
View reviewed changes
Copy link
Contributor

@atscott atscott 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

@atscott
Copy link
Contributor

atscott commented Jan 9, 2024

This PR was merged into the repository by commit 09baed0.

@atscott atscott closed this in 09baed0 Jan 9, 2024
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
@JeanMeche @ChellappanRajan
refactor(core): remove signal mutate implementation (angular#52348)
3db6e65 
It's not used anymore.

PR Close angular#52348
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
@JeanMeche @rlmestre
refactor(core): remove signal mutate implementation (angular#52348)
84b4c4e 
It's not used anymore.

PR Close angular#52348
danieljancar pushed a commit to danieljancar/angular that referenced this pull request Jan 26, 2024
@JeanMeche @danieljancar
refactor(core): remove signal mutate implementation (angular#52348)
4ad638e 
It's not used anymore.

PR Close angular#52348
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
@JeanMeche @amilamen
refactor(core): remove signal mutate implementation (angular#52348)
7e90cea 
It's not used anymore.

PR Close angular#52348
@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 9, 2024
@JeanMeche JeanMeche deleted the mutate branch February 29, 2024 13:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@atscott atscott atscott approved these changes

@pkozlowski-opensource pkozlowski-opensource pkozlowski-opensource approved these changes

@alxhub alxhub Awaiting requested review from alxhub

@dylhunn dylhunn Awaiting requested review from dylhunn

@jessicajaniuk jessicajaniuk Awaiting requested review from jessicajaniuk

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime target: minor This PR is targeted for the next minor release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@JeanMeche @garrettld @pkozlowski-opensource @atscott