Skip to content

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

Merged
merged 1 commit into from
May 23, 2022

Conversation

DeadSix27
Copy link
Contributor

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.

Thank you for changing it to be consistent.

Looks good to me.

@FoamyGuy FoamyGuy merged commit 7f2e03f into adafruit:main May 23, 2022
@FoamyGuy
Copy link
Contributor

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 i2c is passed without the kwarg name so it should be unaffected.

@dhalbert
Copy link

This could be a major version change, to 2.0.0, since it's an API change. (I know you already released it.)

@DeadSix27
Copy link
Contributor Author

DeadSix27 commented May 23, 2022

Technically this would be a breaking change for user code that explicitly uses the argument name rather than relying on

Agreed, was going to mention that too.

Looks good to me.

Thanks

@FoamyGuy
Copy link
Contributor

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?

@DeadSix27
Copy link
Contributor Author

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?

@dhalbert
Copy link

Probably no one will be affected by this anyway, so the 2.0.0 can be deferred to the next release.

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request May 24, 2022
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
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