Lyon changes #27
Lyon changes #27
Conversation
|
I assume #26 should be closed then. Let me know when you need a review. |
|
#26 can indeed be closed. You can review the code now. I left the plotting code in the task because it's quite small and somewhat specific. |
There was a problem hiding this comment.
I must say I do not have enough time for a very thorough review. I am still a bit concerned about how generic this can truly be but the current version appear to reasonably separate concerns. Once my minor comments are addressed and ideally the CIs come back green, I will merge.
exopy_pulses/tasks/tasks/instrs/views/transfer_pulse_loop_task_view.enaml
Outdated
Show resolved
Hide resolved
exopy_pulses/tasks/tasks/instrs/views/transfer_pulse_loop_task_view.enaml
Outdated
Show resolved
Hide resolved
exopy_pulses/tasks/tasks/instrs/views/transfer_sequence_task_view.enaml
Outdated
Show resolved
Hide resolved
| if sequence.context: | ||
| validate_context_driver_pair(view.root.core, | ||
| task.sequence.context, task, view) | ||
| #if sequence.context: |
There was a problem hiding this comment.
Why is it commented out ?
There was a problem hiding this comment.
No idea but according to my colleagues who use the code, it works without this line
There was a problem hiding this comment.
Obviously commenting out checks does not cause crashes right away but this is still suspicious and I would really like to understand why it was deemed necessary.
|
Unfortunately, it seems that Travis fails to build the package so we won't get a green check |
|
Then I guess I am good for fixing the CIs, may take me some time though. Feel free to ping me if nothing happen within a week. |
|
ping @rassouly can you rebase so that we get CIs running ? |
|
Oups, looks like this was not based on the main branch of exopy. I'll need to remove the dependency on |
Since @jerjohste doesn't have time to keep working on PR #26, I'm taking over. I've done a little bit of cleaning up but I still need to add some documentation