Accept unit classes in motor groups#480
Conversation
|
I don't know why that CI error is happening... clearly the files aren't opened as r/w but I don't see why that is different there. |
|
It looks like the first failure is at api_tests.py:144, and the specific error is |
|
Could it be the file permissions are read-only in fake-sys repo? EDIT: nope, those seem to be ok. I am out of ideas for now. |
|
The CI issue should be fixed by #481. |
Can you rebase this on stretch then? |
5e89179 to
192df65
Compare
83e7066 to
09c0f8a
Compare
|
I've rebased both PRs. I think it probably makes the most sense to merge #411 before this one, just to keep the commits somewhat logical. That PR is complete aside from the fact that I'd like to add some info about how to use the new features; I'd appreciate a review of the content over there as well. |
ed3111c to
aa3c12e
Compare
ddemidov
left a comment
There was a problem hiding this comment.
I was out of the loop on this set of changes, and I did not test if it works on the ev3, so I think I'll just let you decide if this is ready to merge.
Some specific things that caught my eye is old-style formatting ('%s' % (v) instead of '{}'.format(v)), but I am not sure how strict are we about these kinds of things.
Also, print is used to spew warnings as opposed to logging module elswhere. We should probably decide on one of those.
I did some things... primarily enabling unit classes in motor groups, plus some other fixes. Also created tests for these things because I don't trust my programming abilities (nor Python).
Targeting this PR on the docs branch because it has other changes to these same files which I don't want to deal with as merge conflicts.
Fixes #474