Skip to content

feedback from pr61 #62

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 3 commits into from
Oct 8, 2023
Merged

feedback from pr61 #62

merged 3 commits into from
Oct 8, 2023

Conversation

FoamyGuy
Copy link
Contributor

@FoamyGuy FoamyGuy commented Jun 12, 2023

Handles all items noted in the comments in #61.

@tekktrik you mentioned in the other thread wanting to take another look after those things were changed. Once you have a chance to do that let me know and I can add any further changes to this branch.

The one change in this that wasn't mentioned was the removal of:

# pylint: disable=consider-using-f-string

I found that pylint is passing even without that so I removed it.

@FoamyGuy FoamyGuy requested a review from tekktrik June 12, 2023 14:39
Copy link
Member

@tekktrik tekktrik left a comment

Choose a reason for hiding this comment

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

One small suggestion I don't think I picked up on originally, but I think this looks good! Thanks for implementing all of these!

@FoamyGuy
Copy link
Contributor Author

FoamyGuy commented Aug 7, 2023

The latest commit adopts the requested change. Take another look when you have a chance @tekktrik

@FoamyGuy FoamyGuy requested a review from tekktrik August 7, 2023 20:37
# Conflicts:
#	adafruit_bme680.py
@FoamyGuy
Copy link
Contributor Author

FoamyGuy commented Oct 8, 2023

I'm going to merge this based on the original 'looks good' other than the addressed change. We can always add or change things in the future if anything else comes up.

@FoamyGuy FoamyGuy merged commit 1f04f54 into adafruit:main Oct 8, 2023
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Oct 9, 2023
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.

2 participants