Skip to content

Using python 3.7 so sphinx is ok with __future__ #12

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 4 commits into from
Jul 10, 2020
Merged

Conversation

evaherrada
Copy link
Collaborator

No description provided.

@evaherrada evaherrada requested a review from dhalbert July 9, 2020 17:04
@evaherrada
Copy link
Collaborator Author

Not sure what's happening here

dhalbert
dhalbert previously approved these changes Jul 9, 2020
@dhalbert
Copy link
Contributor

dhalbert commented Jul 9, 2020

It's trying to run bluetoothctl, which isn't installed on the Action check machine, and probably wouldn't run successfully if it were installed. You could conditionalize that part of the code so it doesn't get run during Actions. @tannewt did this in an old version of the blinka _bleio library:
https://github.com/adafruit/Adafruit_Blinka_bleio/blob/91cd14853fc3541c52e017b9bd44f6dbae8476f3/_bleio.py#L40-L49

@dhalbert dhalbert self-requested a review July 9, 2020 17:53
@dhalbert dhalbert dismissed their stale review July 9, 2020 17:54

Actions error

@evaherrada
Copy link
Collaborator Author

@dhalbert Ok, looks like it's passing now

*reversed(
list(_ble._adapter.address.address_bytes) # pylint: disable=protected-access
# This line causes issues with Sphinx, so we won't run it in the CI
if "GITHUB_ACTION" not in os.environ and "READTHEDOCS" not in os.environ:
Copy link
Contributor

Choose a reason for hiding this comment

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

I steered you a little wrong here. There is no os.environ in CircuitPython, only in regular CPython, so this will throw an exception. So I'd add hasattr(os, "environ") to the beginning of the if`. Also, could you test importing this library on CircuitPython

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, ok. I'll test that as soon as I make that change

@evaherrada evaherrada requested a review from dhalbert July 10, 2020 15:46
Copy link
Contributor

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

Looks good! Thanks for testing.

@dhalbert dhalbert merged commit 82899ad into master Jul 10, 2020
@dhalbert dhalbert deleted the discord-fix branch July 10, 2020 15:51
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Jul 11, 2020
Updating https://github.com/adafruit/Adafruit_CircuitPython_BNO055 to 5.0.2 from 5.0.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_BNO055#50 from adafruit/uart_speed

Updating https://github.com/adafruit/Adafruit_CircuitPython_BLE_BroadcastNet to 0.10.1 from 0.10.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_BLE_BroadcastNet#12 from adafruit/discord-fix
  > Fixed discord invite link

Updating https://github.com/adafruit/Adafruit_CircuitPython_ImageLoad to 0.11.4 from 0.11.3:
  > Fix README rendering

Updating https://github.com/adafruit/Adafruit_CircuitPython_Bundle/circuitpython_library_list.md to NA from NA:
  > Added the following libraries: Adafruit_CircuitPython_ImageLoad
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