Skip to content

Made #14 pass #23

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 10 commits into from
Oct 21, 2019
Merged

Made #14 pass #23

merged 10 commits into from
Oct 21, 2019

Conversation

evaherrada
Copy link
Collaborator

@evaherrada evaherrada commented Oct 20, 2019

@TG-Techie, I'm not sure why sphinx kept failing over that docstring, and I'm even less sure about how just adding a newline would make it pass, but it passes locally now.

This PR is a fix of a really weird issue from #14

@evaherrada evaherrada mentioned this pull request Oct 20, 2019
@ladyada
Copy link
Member

ladyada commented Oct 20, 2019

@dherrada heya can you test this out with some hardware? what displays/eink/LED matricies do you have?

@evaherrada
Copy link
Collaborator Author

@ladyada, I'll do that rn

Copy link
Contributor

@siddacious siddacious left a comment

Choose a reason for hiding this comment

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

a minor nitpick of the original PR; it's a good habit to limit the scope of pylint disables so we're intentional about how we use them.

def draw_char(self, char, x, y, framebuffer, color):
# pylint: disable=too-many-arguments
# pylint: disable=too-many-arguments
def draw_char(self, char, x, y, framebuffer, color, size=1):
Copy link
Contributor

Choose a reason for hiding this comment

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

add the disable to the end of the def line and it will be scoped to this function only. Otherwise put an enable after the function.

@evaherrada evaherrada requested a review from siddacious October 21, 2019 00:06
@siddacious
Copy link
Contributor

Looks bueno.

@siddacious siddacious merged commit f9264dd into adafruit:master Oct 21, 2019
@ladyada
Copy link
Member

ladyada commented Oct 21, 2019

@dherrada did ya get to testing this?

@evaherrada
Copy link
Collaborator Author

@ladyada, yes, I did. The simple test needed some fixing so I did that, but the framebuf module itself was fine

@siddacious
Copy link
Contributor

@ladyada sorry I should have mentioned @dherrada tested it when I merged

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Oct 22, 2019
Updating https://github.com/adafruit/Adafruit_CircuitPython_EPD to 2.5.0 from 2.4.2:
  > Merge pull request adafruit/Adafruit_CircuitPython_EPD#33 from makermelissa/master
  > Merge pull request adafruit/Adafruit_CircuitPython_EPD#32 from makermelissa/master
  > Merge pull request adafruit/Adafruit_CircuitPython_EPD#29 from adafruit/dherrada-patch-1

Updating https://github.com/adafruit/Adafruit_CircuitPython_framebuf to 1.2.0 from 1.1.1:
  > Merge pull request adafruit/Adafruit_CircuitPython_framebuf#24 from boo13/draw-circle
  > Merge pull request adafruit/Adafruit_CircuitPython_framebuf#23 from dherrada/master
  > Merge pull request adafruit/Adafruit_CircuitPython_framebuf#22 from adafruit/dherrada-patch-1

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

4 participants