Skip to content

Add docstrings #19

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Jun 10, 2022
Merged

Add docstrings #19

merged 12 commits into from
Jun 10, 2022

Conversation

tekktrik
Copy link
Member

Resolves #17 by adding docstring documentation, also updates supporting files so that Sphinx will build and show them as documentation.

@tekktrik tekktrik requested a review from dhalbert February 16, 2022 21:52
@tekktrik
Copy link
Member Author

Looks like the asyncio.stream.py has a bunch of yields and returns mixed together, so I can hold off until it's possible for a single commit fix or workaround for them as opposed to doing it one by one

@tekktrik
Copy link
Member Author

Hey @dhalbert, any update on this? I know you're working on asyncio stuff so just wanted to check how this fits in, or if I should expect to update it as you make changes.

@dhalbert
Copy link
Contributor

I think we can proceed on this. It looks like you found the places to change from yield. There are going to be some more changes re #22 but I don't think I will do that immediately.

@tekktrik tekktrik marked this pull request as ready for review March 29, 2022 16:10
@tekktrik
Copy link
Member Author

Sounds good, here it is, ready for review! Let me know if I can help document any additional changes!

FoamyGuy
FoamyGuy requested changes Apr 11, 2022
View reviewed changes
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for imporoving the docs here @tekktrik. I found a few small typos to fix but the majority of this looks good to me.

I noticed a few places where there are real code changes involving await core.sleep(0). I don't have enough experience yet to review those one way or another. But thought it was worth mentioning since they seem to be the only changes outside of docstrings in this PR.

@dhalbert
Copy link
Contributor

Could you test at least some of the examples in the async guide: https://learn.adafruit.com/cooperative-multitasking-in-circuitpython with these changes? Thanks! I will take a final look after that.

@FoamyGuy
Copy link
Contributor

I will test the learn guide examples with this branch in the afternoon after the weekly meeting.

@FoamyGuy
Copy link
Contributor

I tested many of the examples from the learn guide using a Feather RP2040 with some LEDs and buttons wired up. For the neopixel rainbow animation one I used a NeoTrellis and slightly modified the code to read from one row of the matrix.

Everything appears to be functioning as expected.

Ready for review when you have time @dhalbert

@tekktrik
Copy link
Member Author

tekktrik commented Jun 10, 2022 •
edited
Loading

@dhalbert is this still good to go? Looks like it passed the hardware check.

dhalbert
dhalbert approved these changes Jun 10, 2022
View reviewed changes
Copy link
Contributor

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good. Sorry for the delay. Thanks to @FoamyGuy for testing.

@dhalbert dhalbert merged commit c496cfb into adafruit:main Jun 10, 2022
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jun 14, 2022
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@77089da)
fd8e0eb 
Updating https://github.com/adafruit/Adafruit_CircuitPython_DS1307 to 2.1.13 from 2.1.12:
  > Merge pull request adafruit/Adafruit_CircuitPython_DS1307#27 from mew-cx/mewcx_day_of_week_fix

Updating https://github.com/adafruit/Adafruit_CircuitPython_PCF8523 to 1.5.11 from 1.5.10:
  > Merge pull request adafruit/Adafruit_CircuitPython_PCF8523#26 from FoamyGuy/fix_day_names

Updating https://github.com/adafruit/Adafruit_CircuitPython_PCF8563 to 1.0.5 from 1.0.4:
  > Merge pull request adafruit/Adafruit_CircuitPython_PCF8563#5 from FoamyGuy/fix_day_names

Updating https://github.com/adafruit/Adafruit_CircuitPython_asyncio to 0.5.11 from 0.5.10:
  > Merge pull request adafruit/Adafruit_CircuitPython_asyncio#19 from tekktrik/doc/add-docstrings

Updating https://github.com/adafruit/Adafruit_CircuitPython_AzureIoT to 2.5.7 from 2.5.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_AzureIoT#51 from tannewt/update_readme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FoamyGuy FoamyGuy FoamyGuy requested changes

@dhalbert dhalbert dhalbert approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add docstring documentation
3 participants
@tekktrik @dhalbert @FoamyGuy