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

Ensure cache.broadcastWatch passes all relevant WatchOptions to cache.diff #8832

Merged
merged 3 commits into from
Sep 27, 2021

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Sep 23, 2021

As @aj-foster noticed in #8831, canonizeResults (and also returnPartialData!) were missing from this cache.diff call.

Instead of enumerating the missing properties explicitly, we can take advantage of the overlap between WatchOptions and DiffOptions to pass all relevant properties through, so this kind of mistake won't happen again.

Fixes #8831.

@aj-foster
Copy link

Thanks for all of the great (and fast!) work around canonization, including the pivot on the default value. Great job.

Copy link
Member

@hwillson hwillson left a comment

Choose a reason for hiding this comment

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

Great fix - thanks @benjamn!

Both `returnPartialData` and `canonizeResults` were missing from this
call. Instead of enumerating the properties, we can take advantage of
the overlap between `WatchOptions` and `DiffOptions` to pass all
properties through, without having to enumerate them explicitly, so this
kind of mistake won't happen again.

Fixes #8831.
@benjamn benjamn force-pushed the pass-all-relevant-diff-options-in-broadcastWatch branch from eb3c7c8 to 6c5e771 Compare September 27, 2021 13:43
@benjamn benjamn merged commit b7f89f3 into main Sep 27, 2021
@benjamn benjamn deleted the pass-all-relevant-diff-options-in-broadcastWatch branch September 27, 2021 13:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Canonization Options Not Passed via Diff
3 participants