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

Parts and references #330

Merged
merged 6 commits into from May 30, 2020
Merged

Parts and references #330

merged 6 commits into from May 30, 2020

Conversation

amolenaar
Copy link
Member

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Documentation content changes

What is the current behavior?

No parts and references compartment in Block diagram

Issue Number: #138

What is the new behavior?

Parts and references are visible:

image

Does this PR introduce a breaking change?

  • Yes
  • No

NB. It looks like SysML elements are not visible in the tree view at all (!)

@danyeaw danyeaw added the feature A new feature label May 30, 2020
@danyeaw danyeaw added this to the 2.0 milestone May 30, 2020
Copy link
Member

@danyeaw danyeaw left a comment

Choose a reason for hiding this comment

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

This looks really good, bravo! 🎉 I noticed one issue is that when you draw a white diamond association (shared association), it was showing up as a part property when it should be a reference property.

This can be part of another PR, but now that we have the two main types of properties for Blocks, I think we can either de-emphasize or remove completely Attributes and Operations. I guess it is technically possible for a Block to have these, but I don't think I have seen them added in practice. What do you think about us just removing them entirely from the BlockItem and the block property page?

gaphor/SysML/blocks/block.py Outdated Show resolved Hide resolved
gaphor/SysML/blocks/block.py Outdated Show resolved Hide resolved
@amolenaar
Copy link
Member Author

I was a bit in doubt on the shared association. I'll change it accordingly.

@amolenaar
Copy link
Member Author

Removing attributes and operations is trivial now, since Block is a separate diagram item now, not inheriting from ClassItem.

Isn't it custom to have attributes for, for example, maxPSI on an InflationValve and a thresholdPSI on a TirePressureMonitor?

(using American units here, we use Bar as unit for pressure 😃 )

@danyeaw
Copy link
Member

danyeaw commented May 30, 2020

Those would be added by a 3rd Property that we haven't created yet called a Value Property. To support those we would need to add some default ValueTypes and make it possible to add additional ones, since every value property is typed by a ValueType.

@amolenaar
Copy link
Member Author

Then we'd better remove attributes and operations, before they are misused. Or maybe keep attributes until we have ValueTypes in place?

@danyeaw
Copy link
Member

danyeaw commented May 30, 2020

I would be ok with just removing them I think, and then once we get initial SysML support finished, we can come back in with additions like ValueTypes.

@amolenaar
Copy link
Member Author

Fair enough. Sounds like a simple change to me.

@amolenaar
Copy link
Member Author

I removed attributes and operations.

@amolenaar amolenaar merged commit fab8777 into master May 30, 2020
@amolenaar amolenaar deleted the parts-and-references branch May 30, 2020 20:01
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

2 participants