Added components to ProcedureStep and Procedure #132
Conversation
topside/procedures/procedure.py
Outdated
| self.components.add(step.action[0]) | ||
| elif step.action: | ||
| print (step.action) |
Codecov Report
@@ Coverage Diff @@
## master #132 +/- ##
==========================================
+ Coverage 90.96% 91.01% +0.04%
==========================================
Files 31 31
Lines 2403 2414 +11
Branches 394 448 +54
==========================================
+ Hits 2186 2197 +11
Misses 182 182
Partials 35 35
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Mostly looks good, just a quick comment and some formatting stuff :0
Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @jlsajfj, @msubeka, and @wendi-yu)
topside/procedures/procedure.py, line 168 at r1 (raw file):
Previously, jlsajfj wrote…
basically sometimes it'll be a tuple, sometimes it'll be a "StateChangeAction" so we need to account for that
I agree with Joe, but I think we should check for whether our action has a "component" attribute rather than using the tuple accessor of the data class, since this creates a requirement that component has to be the first field if it's in anyAction classes. You should be able to do the check with hasAttr, and then just access the step.action.component field to store it
topside/procedures/procedure.py, line 154 at r3 (raw file):
to last step. Members
total nitpick but there's a bit of whitespace here, I think format.sh doesn't format comments
topside/procedures/tests/test_procedure.py, line 250 at r3 (raw file):
def test_components(): s1 = top.ProcedureStep('s1', ('p1', 'open'), [], 'PRIMARY')
Might also want to check how it behaves with steps in a procedure that refer to the same component - it should be fine with your code right now, but adding the test doesn't hurt
This change is