-
Notifications
You must be signed in to change notification settings - Fork 11
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
Add seq table2cmd #52
Conversation
- convert one seq talbe line into a command to send
77eaad3
to
9a2ec77
Compare
9a2ec77
to
39af918
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #52 +/- ##
==========================================
+ Coverage 96.20% 96.35% +0.14%
==========================================
Files 11 12 +1
Lines 1108 1151 +43
==========================================
+ Hits 1066 1109 +43
Misses 42 42
☔ View full report in Codecov by Sentry. |
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.
Almost there, just a couple of changes need changing in the code originally in the IOC repo
Also, please could you add some entries in the docs for this? |
53d2eae
to
7a70090
Compare
7a70090
to
458b977
Compare
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.
Changes look fine except a couple of minor points.
I would like to see more tests written, for anything that isn't the expected case. I had tests for these cases in the PandAblocks-ioc repo so it seems a shame to lose them.
Another thought: Is it worth trying to use this in PandABlocks-ioc before merging this? That'll check if the API actually is compatible with that project. |
I've now adjusted pandablocks-ioc to use the functions here: PandABlocks/PandABlocks-ioc#15. Everything is passing apart from one test which is broken on dev anyway. |
939d705
to
6a06054
Compare
Closes #49
Adds the functionality missing in #50, making the conversion more general.