feat: Emit migration op event #82
Conversation
| let evaluation = MigrationOpEvaluation { | ||
| key: self.key.clone(), | ||
| value: self.evaluation.value, | ||
| // QUESTION: In the ruby implementation, this can be nil. Why not here? |
There was a problem hiding this comment.
I will answer this question and the following todo in subsequent commits.
| pub reason: Reason, | ||
|
|
||
| #[serde(rename = "variation", skip_serializing_if = "Option::is_none")] | ||
| pub variation_index: Option<VariationIndex>, |
There was a problem hiding this comment.
Too bad Detail encodes as variationIndex instead of variation. Otherwise, we could rely on Detail's serialization directly. 😬
| @@ -1632,6 +1655,431 @@ mod tests { | |||
| assert_eq!(evaluated_stage, stage); | |||
| } | |||
|
|
|||
| #[test_case(Stage::Off, Operation::Read, vec![Origin::Old])] | |||
There was a problem hiding this comment.
It would be nice if we could move more tests to being "integration" tests so we could get them out of this file. Not something for this PR, but just a general sentiment of I wish less stuff was in this file.
There was a problem hiding this comment.
I completely agree. I think the biggest blocker there is we need to build the test data source. Once that is there, testing using only externally visible interfaces is much easier.
|
Did a dependency float this to broken? |
Sure did. Fix is here. |
keelerm84
force-pushed
the
feat/migrations
branch
from
7830380
to
d5db15b
Compare
keelerm84
force-pushed
the
mk/sc-243770/emit-tracker-event
branch
from
ac10311
to
d4c6bcb
Compare
No description provided.