-
Notifications
You must be signed in to change notification settings - Fork 1
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
Review FIXME comments #35
Comments
#37 fixes several of these. Not sure what to do about two of the remaining ones:
Doesn't seem to be a problem there. What am I missing? |
Maybe keep it as a placeholder so we can easily add a test in case we figure out a way to test parts of the submodule without hardware attached. If it doesn't seem necessary now, maybe just change |
If it doesn't cause a problem, maybe just change:
to a clarification (since return size may contradict method name
|
Thanks for showing how you wanted those fixed. Added to #37. |
I searched for "FIXME" again and I think everything is covered if you make that change to the comments. Ready to merge #37 after that? |
More in FIXME and maybe more not? Tests pass, but try examples again. I'm not completely sure how to interpret the output (to determine whether it is as expected). At least one FIXME comment is added in #32, but the comments are regarding code that predates the PR.
The text was updated successfully, but these errors were encountered: