Skip to content
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

Link to CPython docs for subset modules #6341

Merged
merged 7 commits into from
May 3, 2022

Conversation

tekktrik
Copy link
Member

@tekktrik tekktrik commented May 3, 2022

Working towards #6326, adds links to CPython documentation for the following modules:

  • time
  • math
  • random
  • os
  • ssl
  • "socket" - really its socketpool
  • atexit
  • struct
  • traceback

jepler
jepler previously approved these changes May 3, 2022
Copy link
Member

@jepler jepler left a comment

Choose a reason for hiding this comment

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

Thanks! good to merge if green.

Copy link
Collaborator

@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.

Can we do more of these at once? This generates ~3-4 hours of builds.

@tekktrik
Copy link
Member Author

tekktrik commented May 3, 2022

I can add a few basic ones so save time if you'd like! I'll do a bunch that are subsets of CPython here

@jepler
Copy link
Member

jepler commented May 3, 2022

That's great too, feel free to re-request review when you're done.

@tekktrik tekktrik changed the title Link to CPython docs for time module Link to CPython docs for subset modules May 3, 2022
@dhalbert
Copy link
Collaborator

dhalbert commented May 3, 2022

Keep adding as many as you can do - they are easy to review and the fewer builds and merges the better, in this case.

Also each commit restarts the builds, so if you can group changes into one commit or submit commits all at once, it will also save time.

@tekktrik
Copy link
Member Author

tekktrik commented May 3, 2022

Sounds good!

@tekktrik
Copy link
Member Author

tekktrik commented May 3, 2022

Okay @jepler and @dhalbert that's what I can immediately find!

@tekktrik tekktrik requested review from dhalbert and jepler May 3, 2022 18:34
@jepler
Copy link
Member

jepler commented May 3, 2022

You used several different styles of linking, do you want to comment on why?

@tekktrik
Copy link
Member Author

tekktrik commented May 3, 2022

I actually was working on the web interface of github when I noticed the shortcut, so my copypasta wasn't too efficient; let me refactor to use that one.

@tekktrik
Copy link
Member Author

tekktrik commented May 3, 2022

The directive actually came from the .rst files, so I'm glad it works in the source docstrings as well! I'm able to build the docs locally with make html.

@tekktrik
Copy link
Member Author

tekktrik commented May 3, 2022

If there's a standard order we want the note in, let me know (as in, it should be immediately after the module name and before any other information).

See random vs ssl

@tekktrik
Copy link
Member Author

tekktrik commented May 3, 2022

If there is a standard order, I'm also happy to just fix it in another PR that adds more documentation so it doesn't have to be rerun here

shared-bindings/socketpool/__init__.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@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.

Thanks!

@dhalbert
Copy link
Collaborator

dhalbert commented May 3, 2022

I checked off the "done" items in #6326. (No need to edit, you can just click on the checkboxes.)

@dhalbert dhalbert merged commit e5c9f3b into adafruit:main May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants