-
-
Notifications
You must be signed in to change notification settings - Fork 114
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
fix(core) Root hierarchy should have proper inheritance #237
Conversation
@mttrbrts : this is a good test of the new build script, with successful linting and unit testing but regression on coverage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Probably best that we have a quick chat, just to ensure we are aligned.
abstract transaction Transaction {} | ||
abstract participant Participant {} | ||
abstract event Event {}`, 'concerto.cto'); | ||
abstract concept Asset {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to convert all these to Concepts - they should retain their asset etc stereotypes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The are concepts: the new type hierarchy has Assets, Participant, Transaction and Event extensions of Concept. This could be written in two ways:
- as above
- as
abstract transaction Transaction extends Concept {}
I don't think this makes any difference (recall no instance can be created for those since they are abstract) but with the definition nb 2. I was having some weird errors, possibly due to the fact that we are using transaction
before it's actually defined.
@@ -52,7 +52,7 @@ asset Order { | |||
mm.addModelFile( ` | |||
namespace test | |||
|
|||
asset Order { | |||
asset Order identified { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be discussed I think. This now allows assets to not be identifiable - which seems odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just never know which of those are meant to be identified
as a built-in thing or as a user decision. Is it just assets? I'm a little unclear there is a benefit those distinctions, but it may be because I forgot earlier discussions.
I'll play a bit by adding identified
to assets and report.
Signed-off-by: Jerome Simeon <[email protected]>
fc546ee
to
b941281
Compare
Signed-off-by: Jerome Simeon [email protected]
Issue #228
Changes
Event
Transaction
Asset
Participant
back in the type hierarchytimestamp
toEvent
andTransaction
isExplicitlyIdentified
API call sometimes returning undefinedFlags
timestamp
why areevent
antransaction
have a timestamp but the other classes do not? what's the criteria?