-
Notifications
You must be signed in to change notification settings - Fork 10
Rewrite properties documentation #7
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
Conversation
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 following up with this!
adafruit_fona/adafruit_fona.py
Outdated
def gps(self, gps_on=False): | ||
"""Sets GPS module power, parses returned buffer. | ||
"""Enables or disables GPS module. | ||
:param bool gps_on: Enables the GPS module, disabled by default. |
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.
A param here doesn't make sense because it's a setter. The default argument is unneeded because it's always provided.
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.
Also, I think this doc string is ignored in favor of the getter's doc string.
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.
@tannewt Would this be the case with all setters?
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.
Yes
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.
Ok, thanks. Did a pass removing docstrings + parameters for setters.
@brentru Please follow up on this PR. Thanks! |
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.
Changes look good! Thank you!
Updating https://github.com/adafruit/Adafruit_CircuitPython_FONA to 2.1.0 from 2.0.1: > Merge pull request adafruit/Adafruit_CircuitPython_FONA#7 from brentru/update-property-docstrings Updating https://github.com/adafruit/Adafruit_CircuitPython_Requests to 1.7.2 from 1.7.1: > Merge pull request adafruit/Adafruit_CircuitPython_Requests#36 from tannewt/check_send
Rewrite property documentation to treat them like attributes per @tannewt's advice.