-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Remove unused syncDerivedUpdates
action
#62229
Remove unused syncDerivedUpdates
action
#62229
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: -2 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
@@ -1,4 +1,4 @@ | |||
// Keep track of the blocks that should not be pushing an additional | |||
// undo stack when editing the entity. | |||
// See the implementation of `syncDerivedUpdates` and `useBlockSync`. | |||
// See the implementation of `useBlockSync`. | |||
export const undoIgnoreBlocks = new WeakSet(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this variable isn't needed anymore either?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't remove it because it was being used in useBlockSync
. But maybe we can remove these lines as well? You know better than me 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep! They are not used anywhere else and were introduced in the same PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! I just removed it in this commit. Let's see if tests pass.
Flaky tests detected in d76e288. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9363229556
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
* Remove `syncDerivedUpdates` function * Remove `syncDerivedUpdates` mention * Remove unused `undoIgnoreBlocks`
@SantosGuillamot This is my friendly reminder to follow this #62229 (comment) :) |
I'm really sorry for that. Totally my fault. I'll take it into account for future pull requests. And thanks for the reminder. |
* Remove `syncDerivedUpdates` function * Remove `syncDerivedUpdates` mention * Remove unused `undoIgnoreBlocks`
What?
Remove
syncDerivedUpdates
private action.Why?
In this pull request, how pattern overrides are handled was changed and
syncDerivedUpdates
action doesn't seem to be used anymore. I assume it it better to remove it and add it back if needed.How?
Just removing the code.
Testing Instructions
Tests should pass.