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

Add CallBehaviorAction #2495

Merged
merged 1 commit into from
Jul 16, 2023
Merged

Conversation

marek-piirikivi
Copy link
Contributor

@marek-piirikivi marek-piirikivi commented Jul 16, 2023

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bug fix
  • Feature
  • Chore (refactoring, formatting, local variables, other cleanup)
  • Documentation content changes

What is the current behavior?

Issue Number: #2464

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@github-actions github-actions bot added the python Pull requests that update Python code label Jul 16, 2023
@danyeaw
Copy link
Member

danyeaw commented Jul 16, 2023

This is an awesome contribution, thanks @marek-piirikivi! This could be a follow-up PR, but do you think there should be a way to go to the selected Behavior from the CallBehavior?

@danyeaw danyeaw requested a review from amolenaar July 16, 2023 13:37
Copy link
Member

@amolenaar amolenaar left a comment

Choose a reason for hiding this comment

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

Excellent!

The Invocation Actions diagram also looks really nice.

Comment on lines 57 to 61
def name(self):
"""Name conforms to UML2.5.1 16.3.4.1 naming description"""
if self.subject.behavior and not self.subject.name:
return self.subject.behavior.name
return self.subject.name or ""
Copy link
Member

Choose a reason for hiding this comment

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

One thing to improve it even more is to move this code to a format function in gaphor/UML/umlfmt.py. Then the activity name will also be used as fallback in the model browser.

Just noticed this feature is not mentioned in the documentation 🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just noticed that there is a slight conflict with the name formatting when looking at SysMLv1.6. SysML requires behavior name, and optionally you can also include the action name. How should we proceed?

image

Copy link
Member

Choose a reason for hiding this comment

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

We can keep is as is for now.

@marek-piirikivi
Copy link
Contributor Author

Excellent!

The Invocation Actions diagram also looks really nice.

@amolenaar I tried to be as authentic to the UML2.5.1 document and capture everything that seemed relevant. I noticed that Action itself already defines relationship to the InputPin and OutputPin; and so does CallAction+InvocationAction. So, the references are now in uml.py duplicated. 🤔 I am not sure how to address this; or what to do in this instance. Seems as if it is just redefinition of the same thing to use different names for the same thing - in CallAction context, all input pins should be called arguments. What do you think?

@marek-piirikivi
Copy link
Contributor Author

This is an awesome contribution, thanks @marek-piirikivi! This could be a follow-up PR, but do you think there should be a way to go to the selected Behavior from the CallBehavior?

Yes. I think so. It would be a good follow-up.

@amolenaar
Copy link
Member

Excellent!
The Invocation Actions diagram also looks really nice.

@amolenaar I tried to be as authentic to the UML2.5.1 document and capture everything that seemed relevant. I noticed that Action itself already defines relationship to the InputPin and OutputPin; and so does CallAction+InvocationAction. So, the references are now in uml.py duplicated. thinking I am not sure how to address this; or what to do in this instance. Seems as if it is just redefinition of the same thing to use different names for the same thing - in CallAction context, all input pins should be called arguments. What do you think?

I have no clear strategy for those. As long as a relation is not actively used by the application I nowadays tend to leave it out.

The UML meta model is not always consistent. Often a value subsets a value that is not a derived unions; something that our Gaphor model cannot deal with and will ignore.

@amolenaar
Copy link
Member

amolenaar commented Jul 16, 2023

Let's call it a day and merge this. A really well executed PR.

Thanks @marek-piirikivi! 🍻

@amolenaar amolenaar merged commit 24b47e1 into gaphor:main Jul 16, 2023
6 checks passed
@danyeaw danyeaw added feature A new feature and removed python Pull requests that update Python code labels Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants