Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Store consequentEffects and alternateEffects in consequent and alternate #2134

Closed
wants to merge 4 commits into from

Conversation

cblappert
Copy link
Contributor

Release Notes: None

Follow up on #2110, this PR makes progress towards unifying PossiblyNormalCompletion and ForkedAbruptCompletion as well as unifying completion and effects.

Introduces updateConsequentKeepingCurrentEffects for a common pattern of updating one of the completions for a PNC/FAC and fixing up the effects.

Copy link
Contributor

@hermanventer hermanventer left a comment

Choose a reason for hiding this comment

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

Nice start. Still a long way to go.

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@cblappert is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@hermanventer hermanventer deleted the refactor4 branch June 25, 2018 20:05
facebook-github-bot pushed a commit that referenced this pull request Jun 26, 2018
Summary:
Release note: Fix test runner and don't swallow invariant failures

The recent changes to make the serializer test runner deal with promises, caused it to list every test that follows a failing test as also failing.

I've also noticed that invariant failures can get swallowed. And as a result of that some test cases that should have failed with false invariants showed up as passing.

Those failures were recently introduced by PR #2134. It boils down to a single line fix, which is also included with this.
Closes #2155

Differential Revision: D8629298

Pulled By: hermanventer

fbshipit-source-id: 467f7efdf119ea9d29083d4e41054e583b38a1ff
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.

3 participants