-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix constant names and typos in aioble examples and deepcopy function #1058
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
base: master
Are you sure you want to change the base?
Conversation
closes: micropython#1055 Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
closes: micropython#699 Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
closes: micropython#952 Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
This is a working alternative to PR micropython#919 closes: micropython#919 Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
Signed-off-by: Jos Verlinde <Jos_Verlinde@hotmail.com>
dpgeorge
left a comment
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.
Thanks for this, there are some good fixes here.
I approve all commits except the RST formatting one.
| THE SOFTWARE. | ||
| Basic example usage: | ||
| Basic example usage:: |
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.
I'm not against this indenting change, but doe we need to make it RST compatible with the double colon?
I'm curious why you made the change / how you found these problems? It's not really supposed to be RST format.
| @@ -1,4 +1,5 @@ | |||
| # DS18x20 temperature sensor driver for MicroPython. | |||
| """DS18x20 temperature sensor driver for MicroPython.""" | |||
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.
Why this change? If it should be a docstring, then why not also include the below license/copyright line as well?
| 1. End of file on input is processed as the command 'EOF'. | ||
| 2. A command is parsed out of each line by collecting the prefix composed | ||
| 2. A command is parsed out of each line by collecting the prefix composed |
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.
I don't think it should add a space at the end of the line.
| ---------------------------------------------------------------------------- | ||
| This is a copy of python's Cmd, but leaves out features that aren't relevant | ||
| or can't currently be implemented for MicroPython. | ||
| .. caution:: |
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.
I really don't think we should be using RST formatting here.
| heapify(x) # transforms list into a heap, in-place, in linear time | ||
| item = heapreplace(heap, item) # pops and returns smallest item, and adds | ||
| # new item; the heap size is unchanged | ||
| heap = [] # creates an empty heap |
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.
Why not 4 space indent like the other example code reformatted in this PR?
|
The rest formatting was a side effect of the work I started with @jimmo to see if/how it would be possible to generate documentation pages from the source files. With these changes that works, although de demo i built for that is no longer live. It ( sphinx +autoapi) does require consistent formatting, and sphinx and the rest of the project use Rst. It could be simpler markdowns or plain docstrings as well, but not RST with formatting errors. This was discussed in |
This PR combines a few rather minor fixes of typos in documentation and scripts.
Fixes #1055, #699, #952