-
Notifications
You must be signed in to change notification settings - Fork 4
Change i2c bus parameter name to match most other libraries #6
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.
Thank you for changing it to be consistent.
Looks good to me.
Technically this would be a breaking change for user code that explicitly uses the argument name rather than relying on position. I searched the Learn Guide repo and found only a single usage of this library and |
This could be a major version change, to 2.0.0, since it's an API change. (I know you already released it.) |
Agreed, was going to mention that too.
Thanks |
Drat, I initially considered 2.0.0 but ended up changing to 1.1.0 when I made the release. Should have stuck with the first instinct. If I recall correctly making a second release without any further changes to the library causes some trouble with the pip release. Should we try and see if maybe I'm misremembering? Or try to find some small innocuous change and make a new PR from it and then a 2.x release? |
Probably better to just resolve the missing typing (#3) and then simply do a 2.0 release, until then, does it really matter whether its a minor or a major version change? |
Probably no one will be affected by this anyway, so the 2.0.0 can be deferred to the next release. |
Updating https://github.com/adafruit/Adafruit_CircuitPython_BH1750 to 1.1.0 from 1.0.7: > Merge pull request adafruit/Adafruit_CircuitPython_BH1750#6 from DeadSix27/main > Patch .pre-commit-config.yaml > change discord badge > Patch: Replaced discord badge image > Updated gitignore > Update Black to latest. > Fixed readthedocs build > Consolidate Documentation sections of README Updating https://github.com/adafruit/Adafruit_CircuitPython_CircuitPlayground to 5.2.1 from 5.2.0: > Merge pull request adafruit/Adafruit_CircuitPython_CircuitPlayground#117 from Neradoc/frozen-module-subdirectory Updating https://github.com/adafruit/Adafruit_CircuitPython_DS1307 to 2.1.10 from 2.1.9: > Merge pull request adafruit/Adafruit_CircuitPython_DS1307#24 from coplate/main > Patch .pre-commit-config.yaml > change discord badge > Patch: Replaced discord badge image > Updated gitignore > Update Black to latest. > Fixed readthedocs build > Consolidate Documentation sections of README Updating https://github.com/adafruit/Adafruit_CircuitPython_UC8151D to 1.1.0 from 1.0.2: > Merge pull request adafruit/Adafruit_CircuitPython_UC8151D#4 from HDR/main > Patch .pre-commit-config.yaml > change discord badge > Patch: Replaced discord badge image > Updated gitignore > Update Black to latest. > Fixed readthedocs build > Post-patch cleanup > Consolidate Documentation sections of README Updating https://github.com/adafruit/Adafruit_CircuitPython_BitmapSaver to 1.2.1 from 1.2.0: > Merge pull request adafruit/Adafruit_CircuitPython_BitmapSaver#23 from matt-land/add-typing > Patch .pre-commit-config.yaml
In most libraries offered by CircuitPython the i2c bus parameter name is simply i2c, in this library it isn't.
So, change it to i2c to match others and keep it unified.
Notable examples:
https://github.com/adafruit/Adafruit_CircuitPython_ADS1x15/blob/main/adafruit_ads1x15/ads1x15.py#L69=
https://github.com/adafruit/Adafruit_CircuitPython_BME680/blob/main/adafruit_bme680.py#L454=
https://github.com/adafruit/Adafruit_CircuitPython_TCA9548A/blob/main/adafruit_tca9548a.py#L90=