12009 pre build verbosity #25
Conversation
Update with xtuml
Add more path data for INFO logs during action processing.
Remove forrmatting
Add text indicating line number and column
bridgepoint/oal.py
Outdated
| raise ParseException("invalid token '%s' at %s:%s" % (p.type, | ||
| p.lineno, | ||
| find_column(p.lexer.lexdata, p.lexpos))) | ||
| raise ParseException("invalid token '%s' at %s : %s" % (p.type, |
There was a problem hiding this comment.
How about : raise ParseException("invalid token '%s' at line %s, column %s" % (p.type, p.lineno, ... ?)
bridgepoint/prebuild.py
Outdated
| s_ee = one(self._s_brg).S_EE[19]() | ||
| return '%s::%s' % (s_ee.Name, self._s_brg.Name) | ||
|
|
||
| compName = "" |
There was a problem hiding this comment.
pyxtuml does not use camel case, use comp_name instead
bridgepoint/prebuild.py
Outdated
|
|
||
| def __init__(self, metamodel, sm_act): | ||
| self._sm_act = sm_act | ||
| self._assigned = one( |
There was a problem hiding this comment.
self._sm_evt instead of self._assigned?
bridgepoint/prebuild.py
Outdated
| self._sm_act = sm_act | ||
| self._assigned = one( | ||
| sm_act).SM_AH[514].SM_TAH[513].SM_TXN[530].SM_NSTXN[507].SM_SEME[504].SM_SEVT[503].SM_EVT[525]() | ||
| self._state = one(sm_act).SM_AH[514].SM_MOAH[513].SM_STATE[511]() |
There was a problem hiding this comment.
self._sm_state instead of self._state?
Address review comments
bridgepoint/prebuild.py
Outdated
| def label(self): | ||
| return '' | ||
|
|
||
| action_owner_name = "" |
There was a problem hiding this comment.
Is _sm_state always set? If so, this would improve readability:
action_owner_name = self._sm_state.Name
if self._sm_evt:
action_owner_name = self.get_event_name(self._sm_evt)
There was a problem hiding this comment.
It is not, the TransitionPrebuilder covers the case for SM_ACT for both Transitions and States.
bridgepoint/prebuild.py
Outdated
| return '' | ||
|
|
||
| action_owner_name = "" | ||
| if self._assigned is not None: |
There was a problem hiding this comment.
self._assigned was renamed in this commit, updater to new name
bridgepoint/prebuild.py
Outdated
| # return '%s::%s::%s' % (port.Name, interface.Name, self._spr_ro.Name) | ||
| return '' | ||
|
|
||
| interface = one( |
There was a problem hiding this comment.
rename to interface c_i?
bridgepoint/prebuild.py
Outdated
| logger.info('processing action %s' % walker.label) | ||
| walker = walker_map[metaclass.kind]['prebuilder']( | ||
| metaclass.metamodel, instance) | ||
| actionType = walker_map[metaclass.kind]['elementName'] |
Address review comments
bridgepoint/prebuild.py
Outdated
| logger.info('processing action %s' % walker.label) | ||
| walker = walker_map[metaclass.kind]['prebuilder']( | ||
| metaclass.metamodel, instance) | ||
| actionType = walker_map[metaclass.kind]['elementName'] |
There was a problem hiding this comment.
How about moving the action_type as a property to each Prebuilder class, e.g.,
FunctionPrebuilder(ActionPrebuilder):
element_name = 'Function'
|
|
||
| @property | ||
|
|
||
| def get_event_name(self, event): |
There was a problem hiding this comment.
This method is difficult to read and understand.
Could you flatten the conditions a bit, and add comment on each case?
I tried my best to write a sugestion, but I do not have access to the metamodel at the moment.
Here is a possibly incorrect example that points towards my intentions.
def get_event_name(sm_evt):
sm_nlevt = one(sm_evt).SM_SEVT[525].SM_NLEVT[526]()
sm_sgevt = one(sm_evt).SM_SEVT[525].SM_SGEVT[526]()
sm_pect = one(sm_nlevt).SM_PEVT[527]()
# non-local polymorphic event
if sm_pect and sm_pect.SM_ID != sm_nlevt.SM_ID:
return sm_evt.Drv_Lbl + ": " + sm_evt.Mning
# local polymorphic event
if sm_pect:
return sm_evt.Mning + "::" + sm_pect.localClassName
# orphan, what is this?
if sm_nlevt:
return event.Mning + "::Orphaned"
# case 4
if sm_sgevt:
return sm_evt.Drv_Lbl
# case 5
return sm_evt.Drv_Lbl + ": " + sm_evt.Mning
Rework action owner for state/transition. Move element type into each prebuilder.
|
I will merge this when the final comment is addressed (or argued against). Could you provide me with a summarizing comment from the #12009 job description (which is not easily accessible by the general public), and I'll use that as a rebase commit message. Nice job! |
Reformat get_event_name
|
Summarizing comment:
|
|
The other case will work though right? |
|
Actually I have to have it there, maybe I am not following. With it removed there is an error around SM_ID on None. |
|
Okey! 'obj is None' checks if obj is actually equal to None. Lets keep it as your original suggestion, my comment was purely cosmetic... |
|
prebuild: pinpoint error messages to line, column, and type of action. Specifically;
|
|
I messed up with the github merging, had to do a git push --force to update
master with the squash I intended.
I hope you did not get into trouble because of this...
Den ons 7 okt. 2020 kl 22:32 skrev Travis London <notifications@github.com>:
… Actually I have to have it there, maybe I am not following. With it
removed there is an error around SM_ID on None.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#25 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAEFVTVAH3GF6AZUQ5UUPYDSJTF6FANCNFSM4SHZECFQ>
.
|
|
The tests seems to fail, could you have a look at that? See https://travis-ci.org/github/xtuml/pyxtuml/jobs/733791752 for logs. |
|
Please have a look at #27, and make sure it works as expected. |
No description provided.