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

Refactor Dataflow View #102

Merged
merged 1 commit into from
Jun 11, 2024
Merged

Refactor Dataflow View #102

merged 1 commit into from
Jun 11, 2024

Conversation

huyenngn
Copy link
Member

@huyenngn huyenngn commented May 28, 2024

Closes #70

@huyenngn huyenngn changed the title refactor: Code Improvements Refactor Realization View May 28, 2024
@huyenngn huyenngn changed the title Refactor Realization View Refactor Dataflow View May 28, 2024
@huyenngn huyenngn requested a review from Wuestengecko May 30, 2024 07:37
@huyenngn huyenngn marked this pull request as ready for review May 30, 2024 07:37
@huyenngn huyenngn requested review from ewuerger and vik378 as code owners May 30, 2024 07:37
@Wuestengecko
Copy link
Member

There's a bit of overlap between your 3 PRs.

First, there's some confusion about what belongs to refactor-default (#99) and fix-physical-components (#100):

* cbc038b (origin/refactor-default) refactor: Shorter collector function
| * 90099cc (origin/fix-physical-components) feat: Add Physical Links for Context Diagrams
| * 7972e0b test: Add test case
| *   8a1111b merge: Merge better code PR
| |\
| |/
|/|
* | ee52fd4 refactor: Code improvements
| * 01ca54b test: Add test data
|/
* 8634a24 (tag: v0.2.36) Add-owners (#98)

Then there's also ee52fd4 - in both #99 and #100, see above - and 1936ce9 - in here -, which are both described as "refactor: Code improvements", but are vastly different from each other:

--- ee52fd4f8daf719437b1b3d8a8a6d429a8c35e51
+++ 1936ce92930c190f48c0c2c131b7a62d43cb6520
@@ -1,9 +1,8 @@
-commit ee52fd4f8daf719437b1b3d8a8a6d429a8c35e51
+commit 1936ce92930c190f48c0c2c131b7a62d43cb6520
 Author: huyenngn <[email protected]>
-Date:   Wed May 15 16:41:26 2024 +0200
+Date:   Tue May 28 11:53:52 2024 +0200

-    refactor: Code improvements
+    refactor: Code Improvements

- capellambse_context_diagrams/collectors/default.py | 39 +++++++++-------------
- capellambse_context_diagrams/context.py            | 20 +++++------
- 2 files changed, 23 insertions(+), 36 deletions(-)
+ .../collectors/dataflow_view.py                    | 127 ++++++++-------------
+ 1 file changed, 45 insertions(+), 82 deletions(-)

The second part is probably gonna be fine if things just get squashed together as usual, but the first part might be worth addressing regardless, just to decouple the two PRs (if that even makes sense).

@huyenngn huyenngn marked this pull request as draft June 10, 2024 08:55
@huyenngn huyenngn force-pushed the refactor-dataflowview branch from 3990c7e to 297cc6d Compare June 11, 2024 13:22
@huyenngn huyenngn marked this pull request as ready for review June 11, 2024 13:23
@huyenngn huyenngn marked this pull request as draft June 11, 2024 13:30
@ewuerger ewuerger marked this pull request as ready for review June 11, 2024 13:47
@huyenngn huyenngn force-pushed the refactor-dataflowview branch from 6a179dc to f376ebd Compare June 11, 2024 13:54
Copy link
Member

@ewuerger ewuerger left a comment

Choose a reason for hiding this comment

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

LGTM.

@ewuerger ewuerger merged commit add9739 into main Jun 11, 2024
9 checks passed
@ewuerger ewuerger deleted the refactor-dataflowview branch June 11, 2024 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor, Test and Documentation for the DataFlowView
3 participants