Skip to content

Fixing emtpy assignment issue#156

Merged
sebinside merged 4 commits into
mainfrom
converterFix
May 29, 2024
Merged

Fixing emtpy assignment issue#156
sebinside merged 4 commits into
mainfrom
converterFix

Conversation

@01Parzival10

Copy link
Copy Markdown
Contributor

@uuqjz You added an assignment template which was never removed in case of an empty output pin list

@uuqjz

uuqjz commented May 20, 2024

Copy link
Copy Markdown
Collaborator

Wouldnt it be easier to pull behaviour.getAssignment().remove(template); out of the if so its executed everytime?

@uuqjz

uuqjz commented May 20, 2024

Copy link
Copy Markdown
Collaborator

Could you add a test (or just an assert in one of the existing tests) that checks the amount of assignment against the amount of outpins?

@01Parzival10

Copy link
Copy Markdown
Contributor Author

Could you add a test (or just an assert in one of the existing tests) that checks the amount of assignment against the amount of outpins?

Are they necessarily equal? We can have multiple assignments per output pin in the Metamodel. Im not sure whether that can happen with the MicroSeCEnd stuff tho.

Otherwise i could just add a test that checks for assignments without output pins.

@uuqjz

uuqjz commented May 27, 2024

Copy link
Copy Markdown
Collaborator

Ive added a test, please check whether it is to your liking and would fail without the fix.

@sebinside sebinside added this to the 3.0.0 milestone May 28, 2024
@01Parzival10

Copy link
Copy Markdown
Contributor Author

Tests are fixed, ready to merge

@sebinside sebinside left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGNTM

@sebinside sebinside merged commit 308bdbe into main May 29, 2024
@sebinside sebinside deleted the converterFix branch May 29, 2024 07:00
Nicolas-Boltz pushed a commit that referenced this pull request May 28, 2025
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.

3 participants