Conversation
Signed-off-by: ASVLCII <308706627+ASVLCII@users.noreply.github.com>
|
|
|
All checks and EasyCLA are green now. Feel free to merge when you're ready. |
| }; | ||
|
|
||
| it('detects added, removed, modified, and unchanged flows', () => { | ||
| const base = { ...testArchitectures.baseArchitecture, flows: [flow] } as CalmArchitectureSchema; |
There was a problem hiding this comment.
Could we perhaps create new architectures within ./fixtures/diff-test-architectures.json? This is the current convention in these specs, rather than relying on the base architecture. Maybe we could update the base architecture to add flows: [flow] and then create a new architecture based on changed.
There was a problem hiding this comment.
Yeah, that fits the existing specs better. I'll move these cases into diff-test-architectures.json, add the flow to the base fixture, and derive the changed case from it. Keeps the test setup consistent with the rest of the file.
There was a problem hiding this comment.
Agree with Aman's convention point — worth moving before merge so the flow test cases match the rest of the file.
|
I am happy with these changes, I would also want the opinion of Matt Bain on this though. |
|
Sounds good, happy to wait for Matt's review. The fixture move I mentioned is still outstanding on my side. |
rocketstack-matt
left a comment
rocketstack-matt
left a comment
There was a problem hiding this comment.
Design and tests look right — flows are matched by unique-id and correctly wired into hasChanges/summary/JSON output. Two things before merge: this branch conflicts with main (a parallel PR added MetadataDiffResult to the same ArchitectureDiffResult union), and the CLI README's diff section doesn't mention flows yet (it still documents/shows only Nodes/Relationships in the --format summary output).
| controlItemsModified: { [controlId: string]: ControlItemDiffResult }; | ||
| } | ||
|
|
||
| export interface FlowDiffResult { |
There was a problem hiding this comment.
Nit: this is the only composed result interface here without a doc comment (AdrDiffResult/ControlDiffResult above both have one).
This branch has not been deployed
3 participants
Description
Fixes #2998.
calm diffnow includes top-level flows. Flows match byunique-id; a changed flow is reported as modified.Type of Change
Affected Components
cli/)calm/)calm-ai/)calm-hub/)calm-hub-ui/)calm-server/)calm-widgets/)docs/)shared/)calm-plugins/vscode/)Testing
Focused model and shared diff tests pass. The focused commands exit non-zero only because the repository applies global coverage thresholds to a single-file run.
Checklist